From f3811ce63b4f6b862129c94c0e9c5b72e6f4cf05 Mon Sep 17 00:00:00 2001 From: ryan-crabbe-berri Date: Thu, 4 Jun 2026 20:27:58 -0700 Subject: [PATCH 1/2] refactor(ui): shared HTTP client + location-pinned fetch() lint rule (#29723) * refactor(ui): add shared HTTP client and pin raw fetch() to one file Introduce src/lib/http/client.ts, a single typed wrapper that owns the only fetch() in the dashboard. It centralizes the base URL, the auth header, error parsing (deriveErrorMessage), non-2xx -> thrown ApiError, and JSON parsing, and is framework-agnostic (no React) so it can run from client and, later, server components. The base URL, auth header name and the logout side effect are injected through createApiClient. networking.tsx builds one configured apiClient and the 29 functions whose boilerplate maps exactly to the client's default behavior (canonical deriveErrorMessage + handleError + res.json() template) now call it instead of hand-rolling fetch. Names, signatures, return types and error behavior are unchanged; this is a pure refactor that drops ~440 lines. The no-restricted-syntax fetch rule now points at the client and a files: ["src/lib/http/**"] override makes that the only place fetch() is allowed. Re-baselined eslint-suppressions.json: networking.tsx fetch suppressions drop 270 -> 241; no other rule's counts change. The remaining networking.tsx fetches and the ~61 scattered component/hook fetches diverge from the default client behavior (text() error bodies, no res.ok check, no handleError side effect) and stay grandfathered for a follow-up burndown. * fix(ui): make the HTTP client tolerate non-JSON error bodies The non-2xx branch parsed the error body with response.json(), so a gateway returning HTML (502/503 from a reverse proxy) threw a SyntaxError before onError fired or ApiError was built, dropping the user-facing notification. This matched the old per-function behavior, but the client is now the single error path so it is the right place to harden. Read the body as text once, try JSON.parse for the existing deriveErrorMessage path, and fall back to the raw text (or the HTTP status) otherwise. The success path stays strict json() so return types are unchanged. * fix(ui): await the returned apiClient promise in 6 migrated functions The codemod rendered the `return response.json()` tail as `return apiClient.x()` without `await`. Inside the surrounding try/catch that returns an unawaited promise, so the catch never runs and its console.error log is dropped on failure; 4 of the 6 were `return await response.json()` originally, so this restores their exact behavior. Use `return await apiClient.x()` in all six. * refactor(ui): widen onError type and handle empty success bodies Address review notes on the shared client. Type onError as (message: string) => void | Promise so the fire-and-forget async contract (networking passes the async handleError) is explicit rather than silently discarded by void. On the success path, read the body as text and return undefined for an empty body (e.g. a 204 No Content) instead of throwing a SyntaxError, while still parsing non-empty bodies strictly so a malformed JSON response surfaces rather than being masked. Add tests for the 204 case. --- ui/litellm-dashboard/eslint-suppressions.json | 202 ++++++ ui/litellm-dashboard/eslint.config.mjs | 14 + .../src/components/networking.tsx | 614 +++--------------- .../src/lib/http/client.test.ts | 88 +++ ui/litellm-dashboard/src/lib/http/client.ts | 155 +++++ .../tests/fetch-location-rule.test.ts | 30 + 6 files changed, 576 insertions(+), 527 deletions(-) create mode 100644 ui/litellm-dashboard/src/lib/http/client.test.ts create mode 100644 ui/litellm-dashboard/src/lib/http/client.ts create mode 100644 ui/litellm-dashboard/tests/fetch-location-rule.test.ts diff --git a/ui/litellm-dashboard/eslint-suppressions.json b/ui/litellm-dashboard/eslint-suppressions.json index 980104845b5..47d15f416a0 100644 --- a/ui/litellm-dashboard/eslint-suppressions.json +++ b/ui/litellm-dashboard/eslint-suppressions.json @@ -4,11 +4,86 @@ "count": 1 } }, + "src/app/(dashboard)/hooks/accessGroups/useAccessGroupDetails.ts": { + "no-restricted-syntax": { + "count": 1 + } + }, + "src/app/(dashboard)/hooks/accessGroups/useAccessGroups.ts": { + "no-restricted-syntax": { + "count": 1 + } + }, + "src/app/(dashboard)/hooks/accessGroups/useCreateAccessGroup.ts": { + "no-restricted-syntax": { + "count": 1 + } + }, + "src/app/(dashboard)/hooks/accessGroups/useDeleteAccessGroup.ts": { + "no-restricted-syntax": { + "count": 1 + } + }, + "src/app/(dashboard)/hooks/accessGroups/useEditAccessGroup.ts": { + "no-restricted-syntax": { + "count": 1 + } + }, + "src/app/(dashboard)/hooks/blogPosts/useBlogPosts.ts": { + "no-restricted-syntax": { + "count": 1 + } + }, + "src/app/(dashboard)/hooks/cloudzero/useCloudZeroCreate.ts": { + "no-restricted-syntax": { + "count": 1 + } + }, + "src/app/(dashboard)/hooks/cloudzero/useCloudZeroDryRun.ts": { + "no-restricted-syntax": { + "count": 1 + } + }, + "src/app/(dashboard)/hooks/cloudzero/useCloudZeroExport.ts": { + "no-restricted-syntax": { + "count": 1 + } + }, + "src/app/(dashboard)/hooks/cloudzero/useCloudZeroSettings.ts": { + "no-restricted-syntax": { + "count": 3 + } + }, + "src/app/(dashboard)/hooks/configOverrides/hashicorpVaultApi.ts": { + "no-restricted-syntax": { + "count": 4 + } + }, + "src/app/(dashboard)/hooks/guardrails/useRegisterGuardrail.ts": { + "no-restricted-syntax": { + "count": 1 + } + }, + "src/app/(dashboard)/hooks/healthReadiness/useHealthReadinessDetails.ts": { + "no-restricted-syntax": { + "count": 1 + } + }, "src/app/(dashboard)/hooks/keys/useKeyAliases.test.ts": { "react/display-name": { "count": 1 } }, + "src/app/(dashboard)/hooks/keys/useKeys.ts": { + "no-restricted-syntax": { + "count": 1 + } + }, + "src/app/(dashboard)/hooks/keys/useResetKeySpend.ts": { + "no-restricted-syntax": { + "count": 1 + } + }, "src/app/(dashboard)/hooks/models/useModels.ts": { "max-params": { "count": 1 @@ -19,26 +94,76 @@ "count": 1 } }, + "src/app/(dashboard)/hooks/projects/useCreateProject.ts": { + "no-restricted-syntax": { + "count": 1 + } + }, "src/app/(dashboard)/hooks/projects/useDeleteProject.test.ts": { "react/display-name": { "count": 1 } }, + "src/app/(dashboard)/hooks/projects/useDeleteProject.ts": { + "no-restricted-syntax": { + "count": 1 + } + }, "src/app/(dashboard)/hooks/projects/useProjectDetails.test.ts": { "react/display-name": { "count": 1 } }, + "src/app/(dashboard)/hooks/projects/useProjectDetails.ts": { + "no-restricted-syntax": { + "count": 1 + } + }, "src/app/(dashboard)/hooks/projects/useProjects.test.ts": { "react/display-name": { "count": 1 } }, + "src/app/(dashboard)/hooks/projects/useProjects.ts": { + "no-restricted-syntax": { + "count": 1 + } + }, "src/app/(dashboard)/hooks/projects/useUpdateProject.test.ts": { "react/display-name": { "count": 1 } }, + "src/app/(dashboard)/hooks/projects/useUpdateProject.ts": { + "no-restricted-syntax": { + "count": 1 + } + }, + "src/app/(dashboard)/hooks/proxyConfig/useProxyConfig.ts": { + "no-restricted-syntax": { + "count": 2 + } + }, + "src/app/(dashboard)/hooks/router/useRouterFields.ts": { + "no-restricted-syntax": { + "count": 1 + } + }, + "src/app/(dashboard)/hooks/storeModelInDB/useStoreModelInDB.ts": { + "no-restricted-syntax": { + "count": 1 + } + }, + "src/app/(dashboard)/hooks/storeRequestInSpendLogs/useStoreRequestInSpendLogs.ts": { + "no-restricted-syntax": { + "count": 1 + } + }, + "src/app/(dashboard)/hooks/teams/useTeams.ts": { + "no-restricted-syntax": { + "count": 2 + } + }, "src/app/(dashboard)/layout.tsx": { "react-hooks/set-state-in-effect": { "count": 1 @@ -231,6 +356,11 @@ "count": 1 } }, + "src/components/CostTrackingSettings/pricing_calculator/use_multi_cost_estimate.ts": { + "no-restricted-syntax": { + "count": 1 + } + }, "src/components/CostTrackingSettings/provider_discount_table.test.tsx": { "unused-imports/no-unused-imports": { "count": 1 @@ -251,6 +381,16 @@ "count": 1 } }, + "src/components/CostTrackingSettings/use_discount_config.ts": { + "no-restricted-syntax": { + "count": 2 + } + }, + "src/components/CostTrackingSettings/use_margin_config.ts": { + "no-restricted-syntax": { + "count": 2 + } + }, "src/components/CreateUserButton.tsx": { "no-restricted-imports": { "count": 1 @@ -562,6 +702,9 @@ } }, "src/components/WebRTCTester.jsx": { + "no-restricted-syntax": { + "count": 2 + }, "react/no-unescaped-entities": { "count": 2 } @@ -830,6 +973,9 @@ "no-restricted-imports": { "count": 1 }, + "no-restricted-syntax": { + "count": 3 + }, "react-hooks/immutability": { "count": 1 } @@ -1032,6 +1178,9 @@ "no-restricted-imports": { "count": 1 }, + "no-restricted-syntax": { + "count": 1 + }, "react-hooks/set-state-in-effect": { "count": 1 } @@ -1104,6 +1253,11 @@ "count": 1 } }, + "src/components/mcp_tools/ByokCredentialModal.tsx": { + "no-restricted-syntax": { + "count": 1 + } + }, "src/components/mcp_tools/MCPLogoSelector.test.tsx": { "unused-imports/no-unused-imports": { "count": 1 @@ -1348,6 +1502,9 @@ "src/components/networking.tsx": { "max-params": { "count": 23 + }, + "no-restricted-syntax": { + "count": 241 } }, "src/components/object_permissions_view.tsx": { @@ -1464,6 +1621,11 @@ "count": 13 } }, + "src/components/playground/chat_ui/CodeInterpreterOutput.tsx": { + "no-restricted-syntax": { + "count": 2 + } + }, "src/components/playground/chat_ui/CodeInterpreterTool.tsx": { "no-restricted-imports": { "count": 1 @@ -1495,6 +1657,9 @@ "src/components/playground/llm_calls/a2a_send_message.tsx": { "max-params": { "count": 2 + }, + "no-restricted-syntax": { + "count": 2 } }, "src/components/playground/llm_calls/anthropic_messages.tsx": { @@ -1520,6 +1685,14 @@ "src/components/playground/llm_calls/embeddings_api.tsx": { "max-params": { "count": 1 + }, + "no-restricted-syntax": { + "count": 1 + } + }, + "src/components/playground/llm_calls/fetch_agents.tsx": { + "no-restricted-syntax": { + "count": 1 } }, "src/components/playground/llm_calls/image_edits.tsx": { @@ -1535,6 +1708,9 @@ "src/components/playground/llm_calls/interactions_api.tsx": { "max-params": { "count": 1 + }, + "no-restricted-syntax": { + "count": 1 } }, "src/components/playground/llm_calls/responses_api.tsx": { @@ -1728,6 +1904,11 @@ "count": 1 } }, + "src/components/prompts/prompt_editor_view/conversation_panel/useConversation.ts": { + "no-restricted-syntax": { + "count": 1 + } + }, "src/components/prompts/prompt_info.tsx": { "no-restricted-imports": { "count": 1 @@ -1789,6 +1970,11 @@ "count": 1 } }, + "src/components/survey/SurveyModal.tsx": { + "no-restricted-syntax": { + "count": 1 + } + }, "src/components/tag_management/TagTable.tsx": { "no-restricted-imports": { "count": 1 @@ -1851,6 +2037,11 @@ "count": 1 } }, + "src/components/team/useMyTeamMember.ts": { + "no-restricted-syntax": { + "count": 1 + } + }, "src/components/templates/key_edit_view.tsx": { "no-restricted-imports": { "count": 1 @@ -1878,6 +2069,9 @@ "no-restricted-imports": { "count": 1 }, + "no-restricted-syntax": { + "count": 3 + }, "react-hooks/immutability": { "count": 1 } @@ -2029,6 +2223,9 @@ } }, "src/components/workflow_runs/index.tsx": { + "no-restricted-syntax": { + "count": 3 + }, "react-hooks/set-state-in-effect": { "count": 1 } @@ -2038,6 +2235,11 @@ "count": 1 } }, + "src/contexts/ThemeContext.tsx": { + "no-restricted-syntax": { + "count": 1 + } + }, "src/data/claimsCompliancePrompts.ts": { "max-params": { "count": 1 diff --git a/ui/litellm-dashboard/eslint.config.mjs b/ui/litellm-dashboard/eslint.config.mjs index ca635aef1fc..8235b435950 100644 --- a/ui/litellm-dashboard/eslint.config.mjs +++ b/ui/litellm-dashboard/eslint.config.mjs @@ -32,6 +32,14 @@ const eslintConfig = [ "max-depth": ["warn", 4], "max-params": ["error", 4], "max-nested-callbacks": ["error", 4], + "no-restricted-syntax": [ + "error", + { + selector: "CallExpression[callee.name='fetch']", + message: + "Raw fetch() is only allowed in src/lib/http/. Use the shared client (createApiClient / apiClient) from @/lib/http/client instead.", + }, + ], "no-restricted-imports": [ "error", { @@ -45,6 +53,12 @@ const eslintConfig = [ ], }, }, + { + files: ["src/lib/http/**"], + rules: { + "no-restricted-syntax": "off", + }, + }, ]; export default eslintConfig; diff --git a/ui/litellm-dashboard/src/components/networking.tsx b/ui/litellm-dashboard/src/components/networking.tsx index 07385e6b89f..601840cc257 100644 --- a/ui/litellm-dashboard/src/components/networking.tsx +++ b/ui/litellm-dashboard/src/components/networking.tsx @@ -76,6 +76,10 @@ import { UserInfo } from "./view_users/types"; import { EmailEventSettingsResponse, EmailEventSettingsUpdateRequest } from "./email_events/types"; import { jsonFields } from "./common_components/check_openapi_schema"; import NotificationsManager from "./molecules/notifications_manager"; +import { createApiClient, deriveErrorMessage } from "@/lib/http/client"; + +export { deriveErrorMessage }; +export { ApiError } from "@/lib/http/client"; const isLocal = process.env.NODE_ENV === "development"; // In dev, if NEXT_PUBLIC_USE_REWRITES=true the Next.js dev server proxies API calls @@ -417,6 +421,12 @@ export function getGlobalLitellmHeaderName(): string { return globalLitellmHeaderName; } +const apiClient = createApiClient({ + getBaseUrl: getProxyBaseUrl, + getAuthHeaderName: getGlobalLitellmHeaderName, + onError: handleError, +}); + export const makeModelGroupPublic = async (accessToken: string, modelGroups: string[]) => { const url = proxyBaseUrl ? `${proxyBaseUrl}/model_group/make_public` : `/model_group/make_public`; const response = await fetch(url, { @@ -595,26 +605,12 @@ export const getModelCostMapReloadStatus = async (accessToken: string) => { }; export const modelCreateCall = async (accessToken: string, formValues: Model) => { try { - const url = proxyBaseUrl ? `${proxyBaseUrl}/model/new` : `/model/new`; - const response = await fetch(url, { - method: "POST", - headers: { - [globalLitellmHeaderName]: `Bearer ${accessToken}`, - "Content-Type": "application/json", - }, - body: JSON.stringify({ + const data = await apiClient.post(`/model/new`, { + accessToken, + body: { ...formValues, - }), + }, }); - - if (!response.ok) { - const errorData = await response.json(); - const errorMessage = deriveErrorMessage(errorData); - handleError(errorMessage); - throw new Error(errorMessage); - } - - const data = await response.json(); console.log("API Response:", data); // Close any existing messages before showing new ones @@ -633,26 +629,12 @@ export const modelCreateCall = async (accessToken: string, formValues: Model) => export const modelDeleteCall = async (accessToken: string, model_id: string) => { console.log(`model_id in model delete call: ${model_id}`); try { - const url = proxyBaseUrl ? `${proxyBaseUrl}/model/delete` : `/model/delete`; - const response = await fetch(url, { - method: "POST", - headers: { - [globalLitellmHeaderName]: `Bearer ${accessToken}`, - "Content-Type": "application/json", - }, - body: JSON.stringify({ + const data = await apiClient.post(`/model/delete`, { + accessToken, + body: { id: model_id, - }), + }, }); - - if (!response.ok) { - const errorData = await response.json(); - const errorMessage = deriveErrorMessage(errorData); - handleError(errorMessage); - throw new Error(errorMessage); - } - - const data = await response.json(); console.log("API Response:", data); return data; } catch (error) { @@ -669,26 +651,12 @@ export const budgetDeleteCall = async (accessToken: string | null, budget_id: st } try { - const url = proxyBaseUrl ? `${proxyBaseUrl}/budget/delete` : `/budget/delete`; - const response = await fetch(url, { - method: "POST", - headers: { - [globalLitellmHeaderName]: `Bearer ${accessToken}`, - "Content-Type": "application/json", - }, - body: JSON.stringify({ + const data = await apiClient.post(`/budget/delete`, { + accessToken, + body: { id: budget_id, - }), + }, }); - - if (!response.ok) { - const errorData = await response.json(); - const errorMessage = deriveErrorMessage(errorData); - handleError(errorMessage); - throw new Error(errorMessage); - } - - const data = await response.json(); console.log("API Response:", data); return data; } catch (error) { @@ -705,26 +673,12 @@ export const budgetCreateCall = async ( console.log("Form Values in budgetCreateCall:", formValues); // Log the form values before making the API call console.log("Form Values after check:", formValues); - const url = proxyBaseUrl ? `${proxyBaseUrl}/budget/new` : `/budget/new`; - const response = await fetch(url, { - method: "POST", - headers: { - [globalLitellmHeaderName]: `Bearer ${accessToken}`, - "Content-Type": "application/json", - }, - body: JSON.stringify({ + const data = await apiClient.post(`/budget/new`, { + accessToken, + body: { ...formValues, // Include formValues in the request body - }), + }, }); - - if (!response.ok) { - const errorData = await response.json(); - const errorMessage = deriveErrorMessage(errorData); - handleError(errorMessage); - throw new Error(errorMessage); - } - - const data = await response.json(); console.log("API Response:", data); return data; // Handle success - you might want to update some state or UI based on the created key @@ -742,26 +696,12 @@ export const budgetUpdateCall = async ( console.log("Form Values in budgetUpdateCall:", formValues); // Log the form values before making the API call console.log("Form Values after check:", formValues); - const url = proxyBaseUrl ? `${proxyBaseUrl}/budget/update` : `/budget/update`; - const response = await fetch(url, { - method: "POST", - headers: { - [globalLitellmHeaderName]: `Bearer ${accessToken}`, - "Content-Type": "application/json", - }, - body: JSON.stringify({ + const data = await apiClient.post(`/budget/update`, { + accessToken, + body: { ...formValues, // Include formValues in the request body - }), + }, }); - - if (!response.ok) { - const errorData = await response.json(); - const errorMessage = deriveErrorMessage(errorData); - handleError(errorMessage); - throw new Error(errorMessage); - } - - const data = await response.json(); console.log("API Response:", data); return data; // Handle success - you might want to update some state or UI based on the created key @@ -776,26 +716,12 @@ export const invitationCreateCall = async ( userID: string, // Assuming formValues is an object ) => { try { - const url = proxyBaseUrl ? `${proxyBaseUrl}/invitation/new` : `/invitation/new`; - const response = await fetch(url, { - method: "POST", - headers: { - [globalLitellmHeaderName]: `Bearer ${accessToken}`, - "Content-Type": "application/json", - }, - body: JSON.stringify({ + const data = await apiClient.post(`/invitation/new`, { + accessToken, + body: { user_id: userID, // Include formValues in the request body - }), + }, }); - - if (!response.ok) { - const errorData = await response.json(); - const errorMessage = deriveErrorMessage(errorData); - handleError(errorMessage); - throw new Error(errorMessage); - } - - const data = await response.json(); console.log("API Response:", data); return data; // Handle success - you might want to update some state or UI based on the created key @@ -1671,26 +1597,12 @@ export const organizationCreateCall = async ( } } - const url = proxyBaseUrl ? `${proxyBaseUrl}/organization/new` : `/organization/new`; - const response = await fetch(url, { - method: "POST", - headers: { - [globalLitellmHeaderName]: `Bearer ${accessToken}`, - "Content-Type": "application/json", - }, - body: JSON.stringify({ + const data = await apiClient.post(`/organization/new`, { + accessToken, + body: { ...formValues, // Include formValues in the request body - }), + }, }); - - if (!response.ok) { - const errorData = await response.json(); - const errorMessage = deriveErrorMessage(errorData); - handleError(errorMessage); - throw new Error(errorMessage); - } - - const data = await response.json(); console.log("API Response:", data); return data; // Handle success - you might want to update some state or UI based on the created key @@ -1707,26 +1619,12 @@ export const organizationUpdateCall = async ( try { console.log("Form Values in organizationUpdateCall:", formValues); // Log the form values before making the API call - const url = proxyBaseUrl ? `${proxyBaseUrl}/organization/update` : `/organization/update`; - const response = await fetch(url, { - method: "PATCH", - headers: { - [globalLitellmHeaderName]: `Bearer ${accessToken}`, - "Content-Type": "application/json", - }, - body: JSON.stringify({ + const data = await apiClient.patch(`/organization/update`, { + accessToken, + body: { ...formValues, // Include formValues in the request body - }), + }, }); - - if (!response.ok) { - const errorData = await response.json(); - const errorMessage = deriveErrorMessage(errorData); - handleError(errorMessage); - throw new Error(errorMessage); - } - - const data = await response.json(); console.log("Update Team Response:", data); return data; // Handle success - you might want to update some state or UI based on the created key @@ -2423,23 +2321,10 @@ export const updateUsefulLinksCall = async ( useful_links: Record, ) => { try { - const url = proxyBaseUrl ? `${proxyBaseUrl}/model_hub/update_useful_links` : `/model_hub/update_useful_links`; - const response = await fetch(url, { - method: "POST", - headers: { - [globalLitellmHeaderName]: `Bearer ${accessToken}`, - "Content-Type": "application/json", - }, - body: JSON.stringify({ useful_links: useful_links }), + return await apiClient.post(`/model_hub/update_useful_links`, { + accessToken, + body: { useful_links: useful_links }, }); - if (!response.ok) { - const errorData = await response.json(); - const errorMessage = deriveErrorMessage(errorData); - handleError(errorMessage); - throw new Error(errorMessage); - } - - return await response.json(); } catch (error) { console.error("Failed to create key:", error); throw error; @@ -3422,26 +3307,12 @@ export const teamCreateCall = async ( } } - const url = proxyBaseUrl ? `${proxyBaseUrl}/team/new` : `/team/new`; - const response = await fetch(url, { - method: "POST", - headers: { - [globalLitellmHeaderName]: `Bearer ${accessToken}`, - "Content-Type": "application/json", - }, - body: JSON.stringify({ + const data = await apiClient.post(`/team/new`, { + accessToken, + body: { ...formValues, // Include formValues in the request body - }), + }, }); - - if (!response.ok) { - const errorData = await response.json(); - const errorMessage = deriveErrorMessage(errorData); - handleError(errorMessage); - throw new Error(errorMessage); - } - - const data = await response.json(); console.log("API Response:", data); return data; // Handle success - you might want to update some state or UI based on the created key @@ -3467,26 +3338,12 @@ export const credentialCreateCall = async ( } } - const url = proxyBaseUrl ? `${proxyBaseUrl}/credentials` : `/credentials`; - const response = await fetch(url, { - method: "POST", - headers: { - [globalLitellmHeaderName]: `Bearer ${accessToken}`, - "Content-Type": "application/json", - }, - body: JSON.stringify({ + const data = await apiClient.post(`/credentials`, { + accessToken, + body: { ...formValues, // Include formValues in the request body - }), + }, }); - - if (!response.ok) { - const errorData = await response.json(); - const errorMessage = deriveErrorMessage(errorData); - handleError(errorMessage); - throw new Error(errorMessage); - } - - const data = await response.json(); console.log("API Response:", data); return data; // Handle success - you might want to update some state or UI based on the created key @@ -3612,26 +3469,12 @@ export const credentialUpdateCall = async ( } } - const url = proxyBaseUrl ? `${proxyBaseUrl}/credentials/${credentialName}` : `/credentials/${credentialName}`; - const response = await fetch(url, { - method: "PATCH", - headers: { - [globalLitellmHeaderName]: `Bearer ${accessToken}`, - "Content-Type": "application/json", - }, - body: JSON.stringify({ + const data = await apiClient.patch(`/credentials/${credentialName}`, { + accessToken, + body: { ...formValues, // Include formValues in the request body - }), + }, }); - - if (!response.ok) { - const errorData = await response.json(); - const errorMessage = deriveErrorMessage(errorData); - handleError(errorMessage); - throw new Error(errorMessage); - } - - const data = await response.json(); console.log("API Response:", data); return data; // Handle success - you might want to update some state or UI based on the created key @@ -3970,14 +3813,9 @@ export const teamMemberDeleteCall = async ( try { console.log("Form Values in teamMemberAddCall:", formValues); // Log the form values before making the API call - const url = proxyBaseUrl ? `${proxyBaseUrl}/team/member_delete` : `/team/member_delete`; - const response = await fetch(url, { - method: "POST", - headers: { - [globalLitellmHeaderName]: `Bearer ${accessToken}`, - "Content-Type": "application/json", - }, - body: JSON.stringify({ + const data = await apiClient.post(`/team/member_delete`, { + accessToken, + body: { team_id: teamId, ...(formValues.user_email !== undefined && { user_email: formValues.user_email, @@ -3985,17 +3823,8 @@ export const teamMemberDeleteCall = async ( ...(formValues.user_id !== undefined && { user_id: formValues.user_id, }), - }), + }, }); - - if (!response.ok) { - const errorData = await response.json(); - const errorMessage = deriveErrorMessage(errorData); - handleError(errorMessage); - throw new Error(errorMessage); - } - - const data = await response.json(); console.log("API Response:", data); return data; // Handle success - you might want to update some state or UI based on the created key @@ -4877,23 +4706,7 @@ export const getMCPSemanticFilterSettings = async (accessToken: string) => { * Get MCP semantic filter configuration */ try { - const url = proxyBaseUrl ? `${proxyBaseUrl}/get/mcp_semantic_filter_settings` : `/get/mcp_semantic_filter_settings`; - const response = await fetch(url, { - method: "GET", - headers: { - [globalLitellmHeaderName]: `Bearer ${accessToken}`, - "Content-Type": "application/json", - }, - }); - - if (!response.ok) { - const errorData = await response.json(); - const errorMessage = deriveErrorMessage(errorData); - handleError(errorMessage); - throw new Error(errorMessage); - } - - const data = await response.json(); + const data = await apiClient.get(`/get/mcp_semantic_filter_settings`, { accessToken }); return data; } catch (error) { console.error("Failed to get MCP semantic filter settings:", error); @@ -5245,23 +5058,7 @@ export const getGuardrailsUsageLogs = async ( export const getPoliciesList = async (accessToken: string) => { try { - const url = proxyBaseUrl ? `${proxyBaseUrl}/policies/list` : `/policies/list`; - const response = await fetch(url, { - method: "GET", - headers: { - [globalLitellmHeaderName]: `Bearer ${accessToken}`, - "Content-Type": "application/json", - }, - }); - - if (!response.ok) { - const errorData = await response.json(); - const errorMessage = deriveErrorMessage(errorData); - handleError(errorMessage); - throw new Error(errorMessage); - } - - const data = await response.json(); + const data = await apiClient.get(`/policies/list`, { accessToken }); return data; } catch (error) { console.error("Failed to get policies list:", error); @@ -5358,23 +5155,7 @@ export const testPoliciesAndGuardrails = async ( export const getPolicyInfoWithGuardrails = async (accessToken: string, policyName: string) => { try { - const url = proxyBaseUrl ? `${proxyBaseUrl}/policy/info/${policyName}` : `/policy/info/${policyName}`; - const response = await fetch(url, { - method: "GET", - headers: { - [globalLitellmHeaderName]: `Bearer ${accessToken}`, - "Content-Type": "application/json", - }, - }); - - if (!response.ok) { - const errorData = await response.json(); - const errorMessage = deriveErrorMessage(errorData); - handleError(errorMessage); - throw new Error(errorMessage); - } - - const data = await response.json(); + const data = await apiClient.get(`/policy/info/${policyName}`, { accessToken }); return data; } catch (error) { console.error(`Failed to get policy info for ${policyName}:`, error); @@ -5384,23 +5165,7 @@ export const getPolicyInfoWithGuardrails = async (accessToken: string, policyNam export const getPolicyTemplates = async (accessToken: string) => { try { - const url = proxyBaseUrl ? `${proxyBaseUrl}/policy/templates` : `/policy/templates`; - const response = await fetch(url, { - method: "GET", - headers: { - [globalLitellmHeaderName]: `Bearer ${accessToken}`, - "Content-Type": "application/json", - }, - }); - - if (!response.ok) { - const errorData = await response.json(); - const errorMessage = deriveErrorMessage(errorData); - handleError(errorMessage); - throw new Error(errorMessage); - } - - const data = await response.json(); + const data = await apiClient.get(`/policy/templates`, { accessToken }); return data; } catch (error) { console.error("Failed to get policy templates:", error); @@ -5451,28 +5216,14 @@ export const suggestPolicyTemplates = async ( model: string, ) => { try { - const url = proxyBaseUrl ? `${proxyBaseUrl}/policy/templates/suggest` : `/policy/templates/suggest`; - const response = await fetch(url, { - method: "POST", - headers: { - [globalLitellmHeaderName]: `Bearer ${accessToken}`, - "Content-Type": "application/json", - }, - body: JSON.stringify({ + return await apiClient.post(`/policy/templates/suggest`, { + accessToken, + body: { attack_examples: attackExamples.filter((e) => e.trim()), description, model, - }), + }, }); - - if (!response.ok) { - const errorData = await response.json(); - const errorMessage = deriveErrorMessage(errorData); - handleError(errorMessage); - throw new Error(errorMessage); - } - - return response.json(); } catch (error) { console.error("Failed to suggest policy templates:", error); throw error; @@ -5481,27 +5232,13 @@ export const suggestPolicyTemplates = async ( export const testPolicyTemplate = async (accessToken: string, guardrailDefinitions: any[], text: string) => { try { - const url = proxyBaseUrl ? `${proxyBaseUrl}/policy/templates/test` : `/policy/templates/test`; - const response = await fetch(url, { - method: "POST", - headers: { - [globalLitellmHeaderName]: `Bearer ${accessToken}`, - "Content-Type": "application/json", - }, - body: JSON.stringify({ + return await apiClient.post(`/policy/templates/test`, { + accessToken, + body: { guardrail_definitions: guardrailDefinitions, text, - }), + }, }); - - if (!response.ok) { - const errorData = await response.json(); - const errorMessage = deriveErrorMessage(errorData); - handleError(errorMessage); - throw new Error(errorMessage); - } - - return response.json(); } catch (error) { console.error("Failed to test policy template:", error); throw error; @@ -5656,24 +5393,7 @@ export const usageAiChatStream = async ( export const createPolicyCall = async (accessToken: string, policyData: any) => { try { - const url = proxyBaseUrl ? `${proxyBaseUrl}/policies` : `/policies`; - const response = await fetch(url, { - method: "POST", - headers: { - [globalLitellmHeaderName]: `Bearer ${accessToken}`, - "Content-Type": "application/json", - }, - body: JSON.stringify(policyData), - }); - - if (!response.ok) { - const errorData = await response.json(); - const errorMessage = deriveErrorMessage(errorData); - handleError(errorMessage); - throw new Error(errorMessage); - } - - const data = await response.json(); + const data = await apiClient.post(`/policies`, { accessToken, body: policyData }); return data; } catch (error) { console.error("Failed to create policy:", error); @@ -5683,24 +5403,7 @@ export const createPolicyCall = async (accessToken: string, policyData: any) => export const updatePolicyCall = async (accessToken: string, policyId: string, policyData: any) => { try { - const url = proxyBaseUrl ? `${proxyBaseUrl}/policies/${policyId}` : `/policies/${policyId}`; - const response = await fetch(url, { - method: "PUT", - headers: { - [globalLitellmHeaderName]: `Bearer ${accessToken}`, - "Content-Type": "application/json", - }, - body: JSON.stringify(policyData), - }); - - if (!response.ok) { - const errorData = await response.json(); - const errorMessage = deriveErrorMessage(errorData); - handleError(errorMessage); - throw new Error(errorMessage); - } - - const data = await response.json(); + const data = await apiClient.put(`/policies/${policyId}`, { accessToken, body: policyData }); return data; } catch (error) { console.error("Failed to update policy:", error); @@ -5778,24 +5481,10 @@ export const updatePolicyVersionStatus = async ( versionStatus: "published" | "production", ): Promise => { try { - const url = proxyBaseUrl ? `${proxyBaseUrl}/policies/${policyId}/status` : `/policies/${policyId}/status`; - const response = await fetch(url, { - method: "PUT", - headers: { - [globalLitellmHeaderName]: `Bearer ${accessToken}`, - "Content-Type": "application/json", - }, - body: JSON.stringify({ version_status: versionStatus }), + return await apiClient.put(`/policies/${policyId}/status`, { + accessToken, + body: { version_status: versionStatus }, }); - - if (!response.ok) { - const errorData = await response.json(); - const errorMessage = deriveErrorMessage(errorData); - handleError(errorMessage); - throw new Error(errorMessage); - } - - return await response.json(); } catch (error) { console.error("Failed to update policy version status:", error); throw error; @@ -5804,23 +5493,7 @@ export const updatePolicyVersionStatus = async ( export const deletePolicyCall = async (accessToken: string, policyId: string) => { try { - const url = proxyBaseUrl ? `${proxyBaseUrl}/policies/${policyId}` : `/policies/${policyId}`; - const response = await fetch(url, { - method: "DELETE", - headers: { - [globalLitellmHeaderName]: `Bearer ${accessToken}`, - "Content-Type": "application/json", - }, - }); - - if (!response.ok) { - const errorData = await response.json(); - const errorMessage = deriveErrorMessage(errorData); - handleError(errorMessage); - throw new Error(errorMessage); - } - - const data = await response.json(); + const data = await apiClient.delete(`/policies/${policyId}`, { accessToken }); return data; } catch (error) { console.error("Failed to delete policy:", error); @@ -5830,23 +5503,7 @@ export const deletePolicyCall = async (accessToken: string, policyId: string) => export const getPolicyInfo = async (accessToken: string, policyId: string) => { try { - const url = proxyBaseUrl ? `${proxyBaseUrl}/policies/${policyId}` : `/policies/${policyId}`; - const response = await fetch(url, { - method: "GET", - headers: { - [globalLitellmHeaderName]: `Bearer ${accessToken}`, - "Content-Type": "application/json", - }, - }); - - if (!response.ok) { - const errorData = await response.json(); - const errorMessage = deriveErrorMessage(errorData); - handleError(errorMessage); - throw new Error(errorMessage); - } - - const data = await response.json(); + const data = await apiClient.get(`/policies/${policyId}`, { accessToken }); return data; } catch (error) { console.error("Failed to get policy info:", error); @@ -5858,23 +5515,7 @@ export const getPolicyInfo = async (accessToken: string, policyId: string) => { export const getPolicyAttachmentsList = async (accessToken: string) => { try { - const url = proxyBaseUrl ? `${proxyBaseUrl}/policies/attachments/list` : `/policies/attachments/list`; - const response = await fetch(url, { - method: "GET", - headers: { - [globalLitellmHeaderName]: `Bearer ${accessToken}`, - "Content-Type": "application/json", - }, - }); - - if (!response.ok) { - const errorData = await response.json(); - const errorMessage = deriveErrorMessage(errorData); - handleError(errorMessage); - throw new Error(errorMessage); - } - - const data = await response.json(); + const data = await apiClient.get(`/policies/attachments/list`, { accessToken }); return data; } catch (error) { console.error("Failed to get policy attachments list:", error); @@ -5884,24 +5525,7 @@ export const getPolicyAttachmentsList = async (accessToken: string) => { export const createPolicyAttachmentCall = async (accessToken: string, attachmentData: any) => { try { - const url = proxyBaseUrl ? `${proxyBaseUrl}/policies/attachments` : `/policies/attachments`; - const response = await fetch(url, { - method: "POST", - headers: { - [globalLitellmHeaderName]: `Bearer ${accessToken}`, - "Content-Type": "application/json", - }, - body: JSON.stringify(attachmentData), - }); - - if (!response.ok) { - const errorData = await response.json(); - const errorMessage = deriveErrorMessage(errorData); - handleError(errorMessage); - throw new Error(errorMessage); - } - - const data = await response.json(); + const data = await apiClient.post(`/policies/attachments`, { accessToken, body: attachmentData }); return data; } catch (error) { console.error("Failed to create policy attachment:", error); @@ -5943,24 +5567,10 @@ export const testPipelineCall = async ( testMessages: Array<{ role: string; content: string }>, ) => { try { - const url = proxyBaseUrl ? `${proxyBaseUrl}/policies/test-pipeline` : `/policies/test-pipeline`; - const response = await fetch(url, { - method: "POST", - headers: { - [globalLitellmHeaderName]: `Bearer ${accessToken}`, - "Content-Type": "application/json", - }, - body: JSON.stringify({ pipeline, test_messages: testMessages }), + const data = await apiClient.post(`/policies/test-pipeline`, { + accessToken, + body: { pipeline, test_messages: testMessages }, }); - - if (!response.ok) { - const errorData = await response.json(); - const errorMessage = deriveErrorMessage(errorData); - handleError(errorMessage); - throw new Error(errorMessage); - } - - const data = await response.json(); return data; } catch (error) { console.error("Failed to test pipeline:", error); @@ -6001,24 +5611,7 @@ export const resolvePoliciesCall = async ( context: { team_alias?: string; key_alias?: string; model?: string; tags?: string[] }, ) => { try { - const url = proxyBaseUrl ? `${proxyBaseUrl}/policies/resolve` : `/policies/resolve`; - const response = await fetch(url, { - method: "POST", - headers: { - [globalLitellmHeaderName]: `Bearer ${accessToken}`, - "Content-Type": "application/json", - }, - body: JSON.stringify(context), - }); - - if (!response.ok) { - const errorData = await response.json(); - const errorMessage = deriveErrorMessage(errorData); - handleError(errorMessage); - throw new Error(errorMessage); - } - - return await response.json(); + return await apiClient.post(`/policies/resolve`, { accessToken, body: context }); } catch (error) { console.error("Failed to resolve policies:", error); throw error; @@ -6711,24 +6304,7 @@ export const createMCPServer = async ( export const updateMCPServer = async (accessToken: string, formValues: Record) => { try { - const url = proxyBaseUrl ? `${proxyBaseUrl}/v1/mcp/server` : `/v1/mcp/server`; - const response = await fetch(url, { - method: "PUT", - headers: { - [globalLitellmHeaderName]: `Bearer ${accessToken}`, - "Content-Type": "application/json", - }, - body: JSON.stringify(formValues), - }); - - if (!response.ok) { - const errorData = await response.json(); - const errorMessage = deriveErrorMessage(errorData); - handleError(errorMessage); - throw new Error(errorMessage); - } - - return await response.json(); + return await apiClient.put(`/v1/mcp/server`, { accessToken, body: formValues }); } catch (error) { console.error("Failed to update MCP server:", error); throw error; @@ -9303,22 +8879,6 @@ export const perUserAnalyticsCall = async ( } }; -export const deriveErrorMessage = (errorData: any): string => { - const detail = errorData?.detail; - const detailStr = Array.isArray(detail) - ? detail.map((d: any) => d?.msg || JSON.stringify(d)).join("; ") - : typeof detail === "string" - ? detail - : undefined; - return ( - (errorData?.error && - (errorData.error.message || (typeof errorData.error === "string" ? errorData.error : undefined))) || - errorData?.message || - detailStr || - JSON.stringify(errorData) - ); -}; - export interface LoginRequest { username: string; password: string; diff --git a/ui/litellm-dashboard/src/lib/http/client.test.ts b/ui/litellm-dashboard/src/lib/http/client.test.ts new file mode 100644 index 00000000000..772a7b63db1 --- /dev/null +++ b/ui/litellm-dashboard/src/lib/http/client.test.ts @@ -0,0 +1,88 @@ +import { describe, it, expect, vi } from "vitest"; +import { createApiClient, ApiError } from "./client"; + +const okResponse = (data: unknown): Response => + ({ ok: true, status: 200, text: async () => JSON.stringify(data) }) as unknown as Response; + +const emptyResponse = (status: number): Response => ({ ok: true, status, text: async () => "" }) as unknown as Response; + +const errorResponse = (status: number, body: unknown): Response => + ({ ok: false, status, text: async () => JSON.stringify(body) }) as unknown as Response; + +const rawErrorResponse = (status: number, text: string): Response => + ({ ok: false, status, text: async () => text }) as unknown as Response; + +describe("createApiClient", () => { + it("builds the URL from base + path + query and sets the auth + JSON headers", async () => { + const fetchImpl = vi.fn(async () => okResponse({ ok: true })); + const client = createApiClient({ + getBaseUrl: () => "https://proxy.example", + getAuthHeaderName: () => "x-litellm-key", + fetchImpl, + }); + + const result = await client.get("/models", { accessToken: "sk-123", query: { team: "t1", page: 2 } }); + + expect(result).toEqual({ ok: true }); + expect(fetchImpl).toHaveBeenCalledTimes(1); + const [url, init] = fetchImpl.mock.calls[0]; + expect(url).toBe("https://proxy.example/models?team=t1&page=2"); + expect(init).toMatchObject({ method: "GET" }); + expect(init.headers).toEqual({ + "Content-Type": "application/json", + "x-litellm-key": "Bearer sk-123", + }); + expect(init.body).toBeUndefined(); + }); + + it("JSON-serializes the body for writes", async () => { + const fetchImpl = vi.fn(async () => okResponse({})); + const client = createApiClient({ getBaseUrl: () => "", fetchImpl }); + + await client.post("/model/new", { accessToken: "sk", body: { model_name: "gpt" } }); + + const [, init] = fetchImpl.mock.calls[0]; + expect(init.method).toBe("POST"); + expect(init.body).toBe(JSON.stringify({ model_name: "gpt" })); + }); + + it("throws ApiError with the derived message and invokes onError on a non-2xx response", async () => { + const fetchImpl = vi.fn(async () => errorResponse(403, { error: { message: "no access" } })); + const onError = vi.fn(); + const client = createApiClient({ getBaseUrl: () => "", onError, fetchImpl }); + + const promise = client.get("/keys", { accessToken: "sk" }); + + await expect(promise).rejects.toBeInstanceOf(ApiError); + await expect(promise).rejects.toMatchObject({ message: "no access", status: 403 }); + expect(onError).toHaveBeenCalledWith("no access"); + }); + + it("falls back to the raw text body when a non-2xx response is not JSON (e.g. an HTML 502)", async () => { + const fetchImpl = vi.fn(async () => rawErrorResponse(502, "Bad Gateway")); + const onError = vi.fn(); + const client = createApiClient({ getBaseUrl: () => "", onError, fetchImpl }); + + const promise = client.get("/keys", { accessToken: "sk" }); + + await expect(promise).rejects.toMatchObject({ message: "Bad Gateway", status: 502 }); + expect(onError).toHaveBeenCalledWith("Bad Gateway"); + }); + + it("returns undefined for an empty success body (e.g. a 204 No Content)", async () => { + const fetchImpl = vi.fn(async () => emptyResponse(204)); + const client = createApiClient({ getBaseUrl: () => "", fetchImpl }); + + await expect(client.delete("/policies/abc", { accessToken: "sk" })).resolves.toBeUndefined(); + }); + + it("omits the auth header when no token is provided", async () => { + const fetchImpl = vi.fn(async () => okResponse({})); + const client = createApiClient({ getBaseUrl: () => "", getAuthHeaderName: () => "Authorization", fetchImpl }); + + await client.get("/public/info"); + + const [, init] = fetchImpl.mock.calls[0]; + expect(init.headers).toEqual({ "Content-Type": "application/json" }); + }); +}); diff --git a/ui/litellm-dashboard/src/lib/http/client.ts b/ui/litellm-dashboard/src/lib/http/client.ts new file mode 100644 index 00000000000..60d66dde8c2 --- /dev/null +++ b/ui/litellm-dashboard/src/lib/http/client.ts @@ -0,0 +1,155 @@ +/** + * The single HTTP client for the dashboard. This is the only file allowed to + * call fetch() directly (enforced by the no-restricted-syntax lint rule and its + * src/lib/http/** override in eslint.config.mjs). + * + * It is framework-agnostic on purpose (no React, no module-level singletons from + * the component tree) so the same client can run in client components today and + * in server components later. Everything environment-specific (base URL, auth + * header name, the logout side effect) is injected through createApiClient. + */ + +export type HttpMethod = "GET" | "POST" | "PUT" | "DELETE" | "PATCH"; + +export type QueryValue = string | number | boolean | null | undefined; + +export type QueryParams = Record; + +export interface RequestOptions { + /** Bearer token. When present, the auth header is set automatically. */ + accessToken?: string | null; + /** Serialized to JSON unless `rawBody` is provided. */ + body?: unknown; + /** Sent verbatim (FormData, Blob, pre-stringified text); disables JSON handling. */ + rawBody?: BodyInit; + query?: QueryParams; + headers?: Record; + signal?: AbortSignal; +} + +export class ApiError extends Error { + readonly status: number; + readonly body: unknown; + + constructor(message: string, status: number, body: unknown) { + super(message); + this.name = "ApiError"; + this.status = status; + this.body = body; + } +} + +/** + * Best-effort extraction of a human-readable message from a proxy error body. + * Lives here because error parsing is the client's job; networking.tsx re-exports + * it so existing `@/components/networking` import paths keep working. + */ +export const deriveErrorMessage = (errorData: any): string => { + const detail = errorData?.detail; + const detailStr = Array.isArray(detail) + ? detail.map((d: any) => d?.msg || JSON.stringify(d)).join("; ") + : typeof detail === "string" + ? detail + : undefined; + return ( + (errorData?.error && + (errorData.error.message || (typeof errorData.error === "string" ? errorData.error : undefined))) || + errorData?.message || + detailStr || + JSON.stringify(errorData) + ); +}; + +export interface ApiClientConfig { + /** Resolves the API origin at call time (it can change at runtime). */ + getBaseUrl: () => string; + /** Resolves the auth header name at call time. Defaults to "Authorization". */ + getAuthHeaderName?: () => string; + /** Invoked with the derived message right before a non-2xx response throws. Fire-and-forget. */ + onError?: (message: string) => void | Promise; + /** Injectable fetch implementation; defaults to the global. */ + fetchImpl?: typeof fetch; +} + +export interface ApiClient { + request(method: HttpMethod, path: string, options?: RequestOptions): Promise; + get(path: string, options?: RequestOptions): Promise; + post(path: string, options?: RequestOptions): Promise; + put(path: string, options?: RequestOptions): Promise; + delete(path: string, options?: RequestOptions): Promise; + patch(path: string, options?: RequestOptions): Promise; +} + +const appendQuery = (url: string, query: QueryParams | undefined): string => { + if (!query) return url; + const search = new URLSearchParams(); + for (const [key, value] of Object.entries(query)) { + if (value === undefined || value === null) continue; + if (Array.isArray(value)) { + value.forEach((v) => v !== undefined && v !== null && search.append(key, String(v))); + } else { + search.append(key, String(value)); + } + } + const qs = search.toString(); + if (!qs) return url; + return url.includes("?") ? `${url}&${qs}` : `${url}?${qs}`; +}; + +export function createApiClient(config: ApiClientConfig): ApiClient { + const { getBaseUrl, getAuthHeaderName, onError, fetchImpl } = config; + const doFetch = fetchImpl ?? fetch; + + async function request(method: HttpMethod, path: string, options: RequestOptions = {}): Promise { + const { accessToken, body, rawBody, query, headers: extraHeaders, signal } = options; + + const url = appendQuery(`${getBaseUrl()}${path}`, query); + + const headers: Record = {}; + if (rawBody === undefined) { + headers["Content-Type"] = "application/json"; + } + if (accessToken) { + const headerName = getAuthHeaderName ? getAuthHeaderName() : "Authorization"; + headers[headerName] = `Bearer ${accessToken}`; + } + if (extraHeaders) { + Object.assign(headers, extraHeaders); + } + + const init: RequestInit = { method, headers, signal }; + if (rawBody !== undefined) { + init.body = rawBody; + } else if (body !== undefined) { + init.body = JSON.stringify(body); + } + + const response = await doFetch(url, init); + + if (!response.ok) { + const raw = await response.text(); + let errorBody: unknown = raw; + let message: string; + try { + errorBody = JSON.parse(raw); + message = deriveErrorMessage(errorBody); + } catch { + message = raw || `HTTP ${response.status}`; + } + onError?.(message); + throw new ApiError(message, response.status, errorBody); + } + + const text = await response.text(); + return (text ? JSON.parse(text) : undefined) as T; + } + + return { + request, + get: (path, options) => request("GET", path, options), + post: (path, options) => request("POST", path, options), + put: (path, options) => request("PUT", path, options), + delete: (path, options) => request("DELETE", path, options), + patch: (path, options) => request("PATCH", path, options), + }; +} diff --git a/ui/litellm-dashboard/tests/fetch-location-rule.test.ts b/ui/litellm-dashboard/tests/fetch-location-rule.test.ts new file mode 100644 index 00000000000..bd495ad3c40 --- /dev/null +++ b/ui/litellm-dashboard/tests/fetch-location-rule.test.ts @@ -0,0 +1,30 @@ +import { describe, it, expect, beforeAll } from "vitest"; +import { ESLint } from "eslint"; + +const FETCH_CODE = `export const load = async () => {\n const res = await fetch("/api/thing");\n return res.json();\n};\n`; + +const RULE_ID = "no-restricted-syntax"; + +let eslint: ESLint; + +const fetchMessages = async (filePath: string) => { + const [result] = await eslint.lintText(FETCH_CODE, { filePath }); + return result.messages.filter((m) => m.ruleId === RULE_ID); +}; + +describe("location-based fetch() rule", () => { + beforeAll(() => { + eslint = new ESLint(); + }); + + it("flags a raw fetch() in a normal source file", async () => { + const messages = await fetchMessages("src/components/some_feature.tsx"); + expect(messages).toHaveLength(1); + expect(messages[0].message).toMatch(/@\/lib\/http\/client/); + }); + + it("allows fetch() inside src/lib/http/ (the one place it lives)", async () => { + const messages = await fetchMessages("src/lib/http/client.ts"); + expect(messages).toHaveLength(0); + }); +}); From 56aa55b991373632fec89cd5b408605457aa7686 Mon Sep 17 00:00:00 2001 From: yuneng-jiang Date: Thu, 4 Jun 2026 20:40:40 -0700 Subject: [PATCH 2/2] fix(proxy): stop team BYOK model name corruption on model edit (#29731) * fix(proxy): stop team model name corruption on edit (#28382) (#29001) Team-scoped ("Team-BYOK") models store an internal routing key model_name_{team_id}_{uuid} in the model_name column and the user-facing name in model_info.team_public_model_name. The internal name leaked into /v1, /v2, and /model/info responses; the dashboard bound its edit form to it, so any non-rename save (e.g. a TPM tweak) PATCHed the internal name back. The update path then treated it as a rename, overwriting team_public_model_name and rewriting the team's models[] ACL with the mangled string -- breaking team key calls with team_model_access_denied. Two-layer fix: - Read path (root cause): add _translate_model_name_for_response and apply it in model_info_v2 and _get_proxy_model_info so /v1, /v2, and /model/info surface the public name for team-scoped rows. The DB column and router index keep the internal name as the routing key; this is a presentation-layer swap on a shallow copy (never mutates input). - Write path (defense in depth): harden _get_public_model_name so a value matching the internal shape, or a no-op against the current DB column, is never treated as a rename -- for both the top-level model_name and an explicit model_info.team_public_model_name. Tests: regression for the reported scenario, full branch coverage of _get_public_model_name, two internal-shape guard cases, an end-to-end PATCH through _update_team_model_in_db (asserts the team ACL is untouched), and four response-translation cases. 60 passed (model management), 181 passed (proxy server). * fix(ui): key Agent Builder agent selection on model_info.id (#29729) * fix(ui): key Agent Builder agent selection on model_info.id Once team-scoped BYOK models can share a public name (the backend now returns the public name on /model/info instead of the internal routing key), selecting agents by model_name collides. Key selection, create, update and delete on the stable model_info.id instead, falling back to model_name only for config-defined agents that have no id. * fix(ui): add name-match fallback to post-create agent selection If the just-created agent's id is not yet present in the re-fetched list, try matching by name before falling back to the first agent. Addresses greptile review on #29729. --------- Co-authored-by: tushar8408 <32977767+tushar8408@users.noreply.github.com> --- .../model_management_endpoints.py | 42 ++- litellm/proxy/proxy_server.py | 39 ++- .../test_model_management_endpoints.py | 301 ++++++++++++++++++ .../test_team_model_name_translation.py | 178 +++++++++++ .../playground/chat_ui/AgentBuilderView.tsx | 79 +++-- 5 files changed, 604 insertions(+), 35 deletions(-) create mode 100644 tests/test_litellm/proxy/proxy_server/test_team_model_name_translation.py diff --git a/litellm/proxy/management_endpoints/model_management_endpoints.py b/litellm/proxy/management_endpoints/model_management_endpoints.py index 722fcd30033..404ed4491e5 100644 --- a/litellm/proxy/management_endpoints/model_management_endpoints.py +++ b/litellm/proxy/management_endpoints/model_management_endpoints.py @@ -490,9 +490,45 @@ def _get_public_model_name( patch_data: updateDeployment, db_model: Deployment, ) -> str: - """Determine the public model name from patch or existing model.""" - if patch_data.model_name: - return patch_data.model_name + """Determine the public model name from patch or existing model. + + The top-level ``model_name`` is the rename channel. For team-scoped rows + the DB ``model_name`` column holds an internal routing key + (``model_name_{team_id}_{uuid}``), and ``/model/info`` historically leaked + it into the dashboard edit form, so a non-rename save (e.g. a TPM tweak) + would PATCH the internal name and the update path would treat it as a + rename -- overwriting ``team_public_model_name`` and rewriting the team ACL + (see issue #28382). + + Guard against that by ignoring an incoming ``model_name`` that matches the + internal shape, or is a no-op against the current DB column. Anything else + is a genuine rename and wins. We deliberately do NOT read + ``patch_data.model_info.team_public_model_name``: the dashboard passes the + existing ``model_info`` blob through untouched on a rename, so honoring it + would return the OLD public name and silently drop the rename. + + Precedence (highest first): + 1. patch_data.model_name -- a genuine rename: not internal-shape and not a + no-op against db_model.model_name. + 2. db_model.model_info.team_public_model_name -- existing public name. + 3. db_model.model_name -- last-resort fallback for legacy rows. + """ + team_id = (patch_data.model_info.team_id if patch_data.model_info else None) or ( + db_model.model_info.team_id if db_model.model_info else None + ) + + def _is_internal_shape(name: Optional[str]) -> bool: + if team_id is None or not name: + return False + return name.startswith(f"model_name_{team_id}_") + + incoming = patch_data.model_name + if ( + incoming + and not _is_internal_shape(incoming) + and incoming != db_model.model_name + ): + return incoming if db_model.model_info and db_model.model_info.team_public_model_name: return db_model.model_info.team_public_model_name diff --git a/litellm/proxy/proxy_server.py b/litellm/proxy/proxy_server.py index 7aed9ad894a..72423b2a796 100644 --- a/litellm/proxy/proxy_server.py +++ b/litellm/proxy/proxy_server.py @@ -11888,6 +11888,9 @@ async def model_info_v2( # Update total count to include agents search_total_count = len(all_models) + # Translate `model_name` to the public name for team-scoped rows. + all_models = [_translate_model_name_for_response(m) for m in all_models] + return _paginate_models_response( all_models=all_models, page=page, @@ -12322,6 +12325,33 @@ async def model_metrics_exceptions( return {"data": response, "exception_types": list(exception_types)} +def _translate_model_name_for_response(model: dict) -> dict: + """For team-scoped DB rows, replace `model_name` with the public name + in `model_info.team_public_model_name` before returning. The DB column + and the in-memory router index keep the internal mangled name + (`model_name_{team_id}_{uuid}`) as the routing key -- this swap is a + presentation-layer concern. Returns a shallow copy; never mutates. + + Without this swap the internal name leaks into `/v1/model/info` and + `/v2/model/info`, the dashboard binds its edit form to it, and a + non-rename save round-trips the internal name back -- corrupting + `team_public_model_name` and the team ACL (see issue #28382). + """ + if not isinstance(model, dict): + return model + model_info = model.get("model_info") or {} + if not isinstance(model_info, dict): + return model + team_public = model_info.get("team_public_model_name") + team_id = model_info.get("team_id") + if not team_public or not team_id: + return model + current = model.get("model_name") or "" + if not current.startswith(f"model_name_{team_id}_"): + return model + return {**model, "model_name": team_public} + + def _get_proxy_model_info(model: dict) -> dict: # provided model_info in config.yaml model_info = model.get("model_info", {}) @@ -12362,7 +12392,7 @@ def _get_proxy_model_info(model: dict) -> dict: deployment_dict=model, excluded_keys={"litellm_credential_name"} ) - return model + return _translate_model_name_for_response(model) @router.get( @@ -12502,8 +12532,11 @@ async def model_info_v1( # noqa: PLR0915 else: all_models = [] - for in_place_model in all_models: - in_place_model = _get_proxy_model_info(model=in_place_model) + # Reassign each entry: _get_proxy_model_info returns a (possibly new) + # dict via _translate_model_name_for_response, which does NOT mutate in + # place. Binding only the loop variable would drop the public-name swap + # for team-scoped rows and leak the internal routing key (#28382). + all_models = [_get_proxy_model_info(model=model) for model in all_models] verbose_proxy_logger.debug("all_models: %s", all_models) return {"data": all_models} diff --git a/tests/test_litellm/proxy/management_endpoints/test_model_management_endpoints.py b/tests/test_litellm/proxy/management_endpoints/test_model_management_endpoints.py index 85c7c130b36..f16074a049b 100644 --- a/tests/test_litellm/proxy/management_endpoints/test_model_management_endpoints.py +++ b/tests/test_litellm/proxy/management_endpoints/test_model_management_endpoints.py @@ -1129,6 +1129,307 @@ class TestTeamModelUpdate: ) assert "403" in str(exc_info.value) + def test_get_public_model_name_28382_dashboard_echo_preserves_public_name(self): + """Regression for #28382 - a non-rename dashboard PATCH echoes the + internal generated model_name (model_name_{team}_{uuid}) at the top + level. That internal-shape value must be ignored (not treated as a + rename), so _get_public_model_name falls through to the existing public + name instead of overwriting it with the internal one.""" + from litellm.proxy.management_endpoints.model_management_endpoints import ( + _get_public_model_name, + ) + from litellm.types.router import ModelInfo + + db_model = Deployment( + model_name="model_name_test-team_abc123", + litellm_params=LiteLLM_Params(model="azure/gpt-5.2-low-rpm-testing"), + model_info=ModelInfo( + team_id="test-team", + team_public_model_name="gpt-5.2-low-rpm-testing", + ), + ) + patch_data = updateDeployment( + model_name="model_name_test-team_abc123", + model_info=ModelInfo( + team_id="test-team", + team_public_model_name="gpt-5.2-low-rpm-testing", + ), + ) + + assert ( + _get_public_model_name(patch_data=patch_data, db_model=db_model) + == "gpt-5.2-low-rpm-testing" + ) + + def test_get_public_model_name_preserves_db_public_name_when_internal_name_unchanged( + self, + ): + """If patch_data.model_info has no team_public_model_name and + patch_data.model_name equals db_model.model_name (dashboard re-sending + the internal name without touching the public-name field), the + existing db_model.model_info.team_public_model_name must be preserved.""" + from litellm.proxy.management_endpoints.model_management_endpoints import ( + _get_public_model_name, + ) + from litellm.types.router import ModelInfo + + db_model = Deployment( + model_name="model_name_test-team_abc123", + litellm_params=LiteLLM_Params(model="azure/gpt-5.2-low-rpm-testing"), + model_info=ModelInfo( + team_id="test-team", + team_public_model_name="gpt-5.2-low-rpm-testing", + ), + ) + patch_data = updateDeployment( + model_name="model_name_test-team_abc123", + model_info=ModelInfo(team_id="test-team"), + ) + + assert ( + _get_public_model_name(patch_data=patch_data, db_model=db_model) + == "gpt-5.2-low-rpm-testing" + ) + + def test_get_public_model_name_allows_top_level_rename(self): + """A genuine rename via the top-level model_name field (no + patch_data.model_info.team_public_model_name supplied, and the new + name differs from the existing internal db model_name) must still + return the new name.""" + from litellm.proxy.management_endpoints.model_management_endpoints import ( + _get_public_model_name, + ) + from litellm.types.router import ModelInfo + + db_model = Deployment( + model_name="model_name_test-team_abc123", + litellm_params=LiteLLM_Params(model="azure/gpt-5.2-low-rpm-testing"), + model_info=ModelInfo( + team_id="test-team", + team_public_model_name="old-public-name", + ), + ) + patch_data = updateDeployment( + model_name="new-public-name", + model_info=ModelInfo(team_id="test-team"), + ) + + assert ( + _get_public_model_name(patch_data=patch_data, db_model=db_model) + == "new-public-name" + ) + + def test_get_public_model_name_top_level_rename_wins_over_stale_model_info(self): + """Regression (codex review): on a dashboard rename the UI sends the new + name in model_name but passes the existing model_info blob through + untouched -- so it still carries the OLD team_public_model_name. The + top-level rename must win; otherwise _update_existing_team_model_assignment + sees no change, never updates the team ACL, and the rename is silently + dropped while the UI optimistically shows the new name.""" + from litellm.proxy.management_endpoints.model_management_endpoints import ( + _get_public_model_name, + ) + from litellm.types.router import ModelInfo + + db_model = Deployment( + model_name="model_name_team-a_abc123", + litellm_params=LiteLLM_Params(model="azure/gpt-4.1"), + model_info=ModelInfo( + team_id="team-a", team_public_model_name="old-public-name" + ), + ) + patch_data = updateDeployment( + model_name="new-public-name", + model_info=ModelInfo( + team_id="team-a", + team_public_model_name="old-public-name", # stale, untouched by UI + ), + ) + + assert ( + _get_public_model_name(patch_data=patch_data, db_model=db_model) + == "new-public-name" + ) + + def test_get_public_model_name_falls_back_to_db_public_name(self): + """When patch_data carries no name hints at all (neither model_name + nor model_info.team_public_model_name), fall back to the existing + db_model.model_info.team_public_model_name.""" + from litellm.proxy.management_endpoints.model_management_endpoints import ( + _get_public_model_name, + ) + from litellm.types.router import ModelInfo + + db_model = Deployment( + model_name="model_name_test-team_abc123", + litellm_params=LiteLLM_Params(model="azure/gpt-5.2-low-rpm-testing"), + model_info=ModelInfo( + team_id="test-team", + team_public_model_name="gpt-5.2-low-rpm-testing", + ), + ) + patch_data = updateDeployment( + model_info=ModelInfo(team_id="test-team"), + ) + + assert ( + _get_public_model_name(patch_data=patch_data, db_model=db_model) + == "gpt-5.2-low-rpm-testing" + ) + + def test_get_public_model_name_last_resort_returns_db_model_name(self): + """Legacy rows may have no team_public_model_name anywhere; the + function must still return a string (the existing db_model.model_name) + rather than raising.""" + from litellm.proxy.management_endpoints.model_management_endpoints import ( + _get_public_model_name, + ) + from litellm.types.router import ModelInfo + + db_model = Deployment( + model_name="legacy-model", + litellm_params=LiteLLM_Params(model="azure/legacy"), + model_info=ModelInfo(team_id="test-team"), + ) + patch_data = updateDeployment( + model_info=ModelInfo(team_id="test-team"), + ) + + assert ( + _get_public_model_name(patch_data=patch_data, db_model=db_model) + == "legacy-model" + ) + + def test_get_public_model_name_ignores_different_internal_shape_name(self): + """A stale client may PATCH an internal-shaped model_name that does not + equal the current DB column (e.g. a different uuid). It must NOT be + treated as a rename -- fall through to the existing public name.""" + from litellm.proxy.management_endpoints.model_management_endpoints import ( + _get_public_model_name, + ) + from litellm.types.router import ModelInfo + + db_model = Deployment( + model_name="model_name_test-team_realuuid", + litellm_params=LiteLLM_Params(model="azure/gpt-5.2-low-rpm-testing"), + model_info=ModelInfo( + team_id="test-team", + team_public_model_name="gpt-5.2-low-rpm-testing", + ), + ) + patch_data = updateDeployment( + model_name="model_name_test-team_differentuuid", + model_info=ModelInfo(team_id="test-team"), + ) + + assert ( + _get_public_model_name(patch_data=patch_data, db_model=db_model) + == "gpt-5.2-low-rpm-testing" + ) + + def test_get_public_model_name_ignores_internal_shape_patch_public(self): + """If a corrupted row round-trips an internal-shaped value in + model_info.team_public_model_name, it must not be accepted as the + public name -- fall through to the existing db public name.""" + from litellm.proxy.management_endpoints.model_management_endpoints import ( + _get_public_model_name, + ) + from litellm.types.router import ModelInfo + + db_model = Deployment( + model_name="model_name_test-team_realuuid", + litellm_params=LiteLLM_Params(model="azure/gpt-5.2-low-rpm-testing"), + model_info=ModelInfo( + team_id="test-team", + team_public_model_name="gpt-5.2-low-rpm-testing", + ), + ) + patch_data = updateDeployment( + model_info=ModelInfo( + team_id="test-team", + team_public_model_name="model_name_test-team_realuuid", + ), + ) + + assert ( + _get_public_model_name(patch_data=patch_data, db_model=db_model) + == "gpt-5.2-low-rpm-testing" + ) + + @pytest.mark.asyncio + async def test_dashboard_edit_preserves_public_name_and_acl(self): + """End-to-end regression for #28382: PATCH payload shaped like the + dashboard's model-edit form (top-level model_name = internal generated + name, model_info.team_public_model_name = public name) must NOT trigger + a public-name rename, must NOT touch the team ACL, and must serialize + the public name back into model_info.""" + from litellm.proxy.management_endpoints.model_management_endpoints import ( + _update_team_model_in_db, + ) + from litellm.types.router import ModelInfo + + db_model = Deployment( + model_name="model_name_test-team_abc123", + litellm_params=LiteLLM_Params( + model="azure/gpt-5.2-low-rpm-testing", + custom_llm_provider="azure", + ), + model_info=ModelInfo( + id="model-id-123", + team_id="test-team", + team_public_model_name="gpt-5.2-low-rpm-testing", + ), + ) + patch_data = updateDeployment( + model_name="model_name_test-team_abc123", + litellm_params=None, + model_info=ModelInfo( + id="model-id-123", + team_id="test-team", + team_public_model_name="gpt-5.2-low-rpm-testing", + ), + ) + user_api_key_dict = UserAPIKeyAuth( + user_id="test_user", + user_role=LitellmUserRoles.PROXY_ADMIN, + ) + prisma_client = MockPrismaClient(team_exists=True) + + with ( + patch( + "litellm.proxy.proxy_server.premium_user", + True, + ), + patch( + "litellm.proxy.management_endpoints.model_management_endpoints.team_model_add" + ) as mock_team_model_add, + patch( + "litellm.proxy.management_endpoints.model_management_endpoints.team_model_delete" + ) as mock_team_model_delete, + ): + result = await _update_team_model_in_db( + db_model=db_model, + patch_data=patch_data, + user_api_key_dict=user_api_key_dict, + prisma_client=prisma_client, # type: ignore + ) + + # team ACL must not be touched on a no-op edit + mock_team_model_add.assert_not_called() + mock_team_model_delete.assert_not_called() + + # the merged model_info written to the DB must keep the public name + model_info_json = result.get("model_info", "") + parsed_model_info = json.loads(model_info_json) + assert ( + parsed_model_info.get("team_public_model_name") == "gpt-5.2-low-rpm-testing" + ) + + # the internal model_name must not have been overwritten (caller + # intentionally clears patch_data.model_name so the DB row's name + # column is left alone) + assert result.get("model_name") == "model_name_test-team_abc123" + class TestModelInfoEndpoint: """Test the model_info endpoint for retrieving individual model information""" diff --git a/tests/test_litellm/proxy/proxy_server/test_team_model_name_translation.py b/tests/test_litellm/proxy/proxy_server/test_team_model_name_translation.py new file mode 100644 index 00000000000..97e5c494916 --- /dev/null +++ b/tests/test_litellm/proxy/proxy_server/test_team_model_name_translation.py @@ -0,0 +1,178 @@ +"""Coverage for team-scoped model-name translation in /model/info responses. + +These live in tests/test_litellm/proxy/proxy_server/ (not the top-level +test_proxy_server.py) because the CI coverage job collects this directory. +They exercise the read-path fix for issue #28382: `/v1`, `/v2`, and +`/model/info` must surface `model_info.team_public_model_name` for team-scoped +rows instead of the internal routing key `model_name_{team_id}_{uuid}`. +""" + +from __future__ import annotations + +from unittest.mock import AsyncMock, MagicMock + +import pytest + +import litellm.proxy.proxy_server as ps +from litellm.proxy._types import LitellmUserRoles, UserAPIKeyAuth +from litellm.proxy.proxy_server import ( + _get_proxy_model_info, + _translate_model_name_for_response, +) + + +def _team_row() -> dict: + return { + "model_name": "model_name_team-abc-123_4a6b8", + "litellm_params": {"model": "azure/gpt-5.2-low-rpm-testing"}, + "model_info": { + "id": "byok-id-1", + "team_id": "team-abc-123", + "team_public_model_name": "team-claude-sonnet", + "db_model": True, + }, + } + + +def test_translate_swaps_internal_name_for_public(): + """Team-scoped row: model_name is swapped to the public name.""" + result = _translate_model_name_for_response(_team_row()) + assert result["model_name"] == "team-claude-sonnet" + + +def test_translate_leaves_global_row_untouched(): + """No team_id / team_public_model_name -> pass through unchanged.""" + model = { + "model_name": "gpt-4o", + "litellm_params": {"model": "gpt-4o"}, + "model_info": {"id": "normal-id-1", "db_model": False}, + } + assert _translate_model_name_for_response(model)["model_name"] == "gpt-4o" + + +def test_translate_leaves_non_internal_shape_untouched(): + """Team row whose model_name is not the internal routing key is not rewritten.""" + model = _team_row() + model["model_name"] = "already-public-name" + assert ( + _translate_model_name_for_response(model)["model_name"] == "already-public-name" + ) + + +def test_translate_handles_missing_or_non_dict_model_info(): + """Missing / None / non-dict model_info, and a non-dict model, must not raise.""" + # missing model_info + assert _translate_model_name_for_response({"model_name": "x"})["model_name"] == "x" + # model_info is None -> coerced to {} -> no team fields + assert ( + _translate_model_name_for_response({"model_name": "x", "model_info": None})[ + "model_name" + ] + == "x" + ) + # model_info is a truthy non-dict (e.g. a stray string) -> early return + assert ( + _translate_model_name_for_response( + {"model_name": "x", "model_info": "garbage"} + )["model_name"] + == "x" + ) + # model itself is not a dict + assert _translate_model_name_for_response("not-a-dict") == "not-a-dict" # type: ignore[arg-type] + + +def test_translate_does_not_mutate_input(): + """Returns a shallow copy; the router's in-memory list keeps the routing key.""" + model = _team_row() + result = _translate_model_name_for_response(model) + assert result is not model + assert model["model_name"] == "model_name_team-abc-123_4a6b8" + + +def test_get_proxy_model_info_returns_public_name_for_team_row(): + """`_get_proxy_model_info` must return the public name for a team-scoped + row. Because _translate_model_name_for_response returns a shallow copy + (it does not mutate), callers MUST use the return value -- the + `/v1/model/info` list path historically discarded it, leaking the internal + routing key (#28382).""" + # Mirror the (fixed) /v1/model/info list path: assign the return back. + all_models = [_get_proxy_model_info(model=m) for m in [_team_row()]] + assert all_models[0]["model_name"] == "team-claude-sonnet" + + +@pytest.mark.asyncio +async def test_model_info_v2_translates_team_model_name(monkeypatch): + """/v2/model/info must surface the public name for team-scoped rows. + Covers the translation step in model_info_v2 (the read-path call site).""" + router = MagicMock() + router.model_list = [_team_row()] + + monkeypatch.setattr(ps, "llm_router", router) + monkeypatch.setattr(ps, "prisma_client", MagicMock()) + monkeypatch.setattr(ps.proxy_config, "get_config", AsyncMock(return_value={})) + monkeypatch.setattr( + ps, + "_apply_search_filter_to_models", + AsyncMock(side_effect=lambda all_models, **kw: (all_models, len(all_models))), + ) + monkeypatch.setattr( + ps, "_enrich_model_info_with_litellm_data", lambda model, **kw: model + ) + import litellm.proxy.agent_endpoints.model_list_helpers as mlh + + monkeypatch.setattr( + mlh, + "append_agents_to_model_info", + AsyncMock(side_effect=lambda models, **kw: models), + ) + + admin = UserAPIKeyAuth(user_id="u", user_role=LitellmUserRoles.PROXY_ADMIN) + # Pass every query param explicitly: called directly (not through FastAPI), + # the fastapi.Query(...) defaults are Query objects, not their values. + resp = await ps.model_info_v2( + user_api_key_dict=admin, + model=None, + user_models_only=False, + include_team_models=False, + debug=False, + page=1, + size=50, + search=None, + modelId=None, + teamId=None, + sortBy=None, + sortOrder="asc", + ) + + names = [m["model_name"] for m in resp["data"]] + assert "team-claude-sonnet" in names + assert "model_name_team-abc-123_4a6b8" not in names + + +@pytest.mark.asyncio +async def test_model_info_v1_list_path_translates_team_model_name(monkeypatch): + """/v1/model/info list path (no litellm_model_id) must surface the public + name. Covers the list comprehension that assigns _get_proxy_model_info's + return back into all_models (#28382 review).""" + router = MagicMock() + router.get_model_names.return_value = ["team-claude-sonnet"] + router.get_model_access_groups.return_value = {} + router.get_model_list.return_value = [_team_row()] + + monkeypatch.setattr(ps, "user_model", None) + monkeypatch.setattr(ps, "llm_model_list", [_team_row()]) + monkeypatch.setattr(ps, "llm_router", router) + monkeypatch.setattr(ps, "get_key_models", lambda **kw: []) + monkeypatch.setattr(ps, "get_team_models", lambda **kw: []) + monkeypatch.setattr( + ps, "get_complete_model_list", lambda **kw: ["team-claude-sonnet"] + ) + + admin = UserAPIKeyAuth( + user_id="u", user_role=LitellmUserRoles.PROXY_ADMIN, team_models=[] + ) + resp = await ps.model_info_v1(user_api_key_dict=admin, litellm_model_id=None) + + names = [m["model_name"] for m in resp["data"]] + assert "team-claude-sonnet" in names + assert "model_name_team-abc-123_4a6b8" not in names diff --git a/ui/litellm-dashboard/src/components/playground/chat_ui/AgentBuilderView.tsx b/ui/litellm-dashboard/src/components/playground/chat_ui/AgentBuilderView.tsx index 97e2af2ed09..8b0de5dda7c 100644 --- a/ui/litellm-dashboard/src/components/playground/chat_ui/AgentBuilderView.tsx +++ b/ui/litellm-dashboard/src/components/playground/chat_ui/AgentBuilderView.tsx @@ -131,6 +131,14 @@ function getAgentModelId(agent: AgentModel): string | null { return info?.id ?? null; } +// Selection key that always resolves to a non-null string. Prefers the DB +// id (stable across renames and unique across teams) but falls back to +// `model_name` so config-file-defined agents — which have no `model_info.id` +// — remain selectable. +function getAgentSelectionKey(agent: AgentModel): string { + return getAgentModelId(agent) ?? agent.model_name; +} + function parseUnderlyingModel(litellmModel: string | undefined): string | undefined { if (!litellmModel || !litellmModel.startsWith("litellm_agent/")) return undefined; return litellmModel.slice("litellm_agent/".length) || undefined; @@ -196,22 +204,24 @@ export default function AgentBuilderView({ const effectiveApiKey = apiKey || accessToken || ""; const selectedAgent = - selectedId === NEW_AGENT_ID ? null : agentModels.find((a) => a.model_name === selectedId) ?? null; + selectedId === NEW_AGENT_ID ? null : agentModels.find((a) => getAgentSelectionKey(a) === selectedId) ?? null; const isNewAgent = selectedId === NEW_AGENT_ID; const selectedAgentModelId = selectedAgent ? getAgentModelId(selectedAgent) : null; - const loadAgents = useCallback(async () => { - if (!accessToken || !userID || !userRole) return; + const loadAgents = useCallback(async (): Promise => { + if (!accessToken || !userID || !userRole) return []; setLoadingAgents(true); try { const list = await fetchAvailableAgentModels(accessToken, userID, userRole); setAgentModels(list); - if (!selectedId || (selectedId !== NEW_AGENT_ID && !list.some((a) => a.model_name === selectedId))) { - setSelectedId(list.length > 0 ? list[0].model_name : null); + if (!selectedId || (selectedId !== NEW_AGENT_ID && !list.some((a) => getAgentSelectionKey(a) === selectedId))) { + setSelectedId(list.length > 0 ? getAgentSelectionKey(list[0]) : null); } + return list; } catch (e) { console.error(e); NotificationsManager.fromBackend("Failed to load agents"); + return []; } finally { setLoadingAgents(false); } @@ -308,7 +318,7 @@ export default function AgentBuilderView({ } setSaving(true); try { - await modelCreateCall(accessToken, { + const response = await modelCreateCall(accessToken, { model_name: draftName.trim(), litellm_params: { model: `litellm_agent/${draftUnderlyingModel}`, @@ -319,9 +329,15 @@ export default function AgentBuilderView({ }, model_info: {}, }); - const newName = draftName.trim(); - await loadAgents(); - setSelectedId(newName); + // /model/new returns the row with `model_id` at the top level. + // Prefer that id over name-matching so we land on the just-created + // agent even when its public name collides with another team's. + const createdId: string | null = response?.model_id ?? response?.model_info?.id ?? null; + const list = await loadAgents(); + const created = createdId + ? list.find((a) => getAgentModelId(a) === createdId) ?? list.find((a) => a.model_name === draftName.trim()) + : list.find((a) => a.model_name === draftName.trim()); + setSelectedId(created ? getAgentSelectionKey(created) : list[0] ? getAgentSelectionKey(list[0]) : null); setActiveTab("chat"); } catch (e) { NotificationsManager.fromBackend("Failed to save agent"); @@ -353,8 +369,10 @@ export default function AgentBuilderView({ selectedAgentModelId, ); NotificationsManager.success("Agent updated successfully"); - await loadAgents(); - setSelectedId(draftName.trim()); + const list = await loadAgents(); + const stillSelected = list.find((a) => getAgentModelId(a) === selectedAgentModelId); + const target = stillSelected ?? list[0]; + setSelectedId(target ? getAgentSelectionKey(target) : null); } catch (e) { NotificationsManager.fromBackend("Failed to update agent"); } finally { @@ -398,9 +416,9 @@ export default function AgentBuilderView({ try { await modelDeleteCall(accessToken, selectedAgentModelId); NotificationsManager.success("Agent deleted"); - await loadAgents(); - const remaining = agentModels.filter((a) => a.model_name !== selectedAgent.model_name); - setSelectedId(remaining.length > 0 ? remaining[0].model_name : null); + const list = await loadAgents(); + const remaining = list.filter((a) => getAgentModelId(a) !== selectedAgentModelId); + setSelectedId(remaining.length > 0 ? getAgentSelectionKey(remaining[0]) : null); } catch (e) { NotificationsManager.fromBackend("Failed to delete agent"); } finally { @@ -462,21 +480,24 @@ export default function AgentBuilderView({ ) : ( <> - {agentModels.map((agent) => ( - - ))} + {agentModels.map((agent) => { + const key = getAgentSelectionKey(agent); + return ( + + ); + })}