From 931b617a51f775327cde2d2cbb75dc3ce0873e55 Mon Sep 17 00:00:00 2001 From: Tin Date: Fri, 10 Jul 2026 00:45:38 -0700 Subject: [PATCH 01/99] feat(mcp): persist admin-entered OAuth app credentials for the client-forwarded modes --- .../mcp_tools/PassthroughAuthorizeSection.tsx | 31 ++++---- .../mcp_tools/create_mcp_server.test.tsx | 71 ++++++++++++++++++- .../mcp_tools/create_mcp_server.tsx | 4 +- .../mcp_tools/mcp_server_edit.test.tsx | 46 ++++++++++++ .../components/mcp_tools/mcp_server_edit.tsx | 4 +- 5 files changed, 138 insertions(+), 18 deletions(-) diff --git a/ui/litellm-dashboard/src/components/mcp_tools/PassthroughAuthorizeSection.tsx b/ui/litellm-dashboard/src/components/mcp_tools/PassthroughAuthorizeSection.tsx index af81f2713ae..314b45fff6c 100644 --- a/ui/litellm-dashboard/src/components/mcp_tools/PassthroughAuthorizeSection.tsx +++ b/ui/litellm-dashboard/src/components/mcp_tools/PassthroughAuthorizeSection.tsx @@ -11,13 +11,14 @@ interface PassthroughOAuthFlow { /** * Browser-only Authorize & Fetch for the client-forwarded token modes - * (true_passthrough / oauth_delegate). LiteLLM never stores upstream - * credentials for these modes, so the token obtained here lives in this - * browser session only: it is forwarded per-server for the tools preview and - * allowlist configuration, and is never written to the server row or the - * per-user credential store. The optional client credentials cover IdPs - * without dynamic client registration (e.g. a pre-registered Slack app) and - * ride the temporary authorize session only. + * (true_passthrough / oauth_delegate). Tokens are never stored: the token + * obtained here lives in this browser session only, forwarded per-server for + * the tools preview and allowlist configuration, and is never written to the + * server row or the per-user credential store. The optional OAuth client + * credentials cover IdPs without dynamic client registration (e.g. a + * pre-registered Slack app); unlike the token they ARE saved onto the server + * as declared config, so internal users' Authorize relays through the org's + * app instead of dead-ending on upstreams that cannot mint clients. */ export default function PassthroughAuthorizeSection({ authType, @@ -35,14 +36,15 @@ export default function PassthroughAuthorizeSection({ return (

- Callers bring their own upstream token for this auth type, so LiteLLM stores no upstream credentials. To preview - tools and configure the tool allowlist, authorize against the upstream here: the token stays in this browser - session only and is never saved to LiteLLM. + Callers bring their own upstream token for this auth type, so LiteLLM never stores tokens. To preview tools and + configure the tool allowlist, authorize against the upstream here: the token stays in this browser session only + and is never saved to LiteLLM. An OAuth app configured below IS saved with the server, so internal users who + authorize from the Tools page go through it.

OAuth Client ID (optional, not saved)} + label={OAuth Client ID (optional, saved)} name={["credentials", "client_id"]} - extra="Only needed when the upstream does not support dynamic client registration (e.g. a pre-registered Slack app). Used for this browser authorization only." + extra="Set this to make everyone authorize through a specific app; required for upstreams without dynamic client registration (e.g. a pre-registered Slack app)." > OAuth Client Secret (optional, not saved)} + label={OAuth Client Secret (optional, saved)} name={["credentials", "client_secret"]} > {oauthFlow.error}

} {oauthFlow.status === "success" && oauthFlow.tokenResponse?.access_token && (

- Token held for this browser session. Tools can now be previewed and configured; nothing was saved to LiteLLM. + Token held for this browser session. Tools can now be previewed and configured; the token was not saved to + LiteLLM.

)}
diff --git a/ui/litellm-dashboard/src/components/mcp_tools/create_mcp_server.test.tsx b/ui/litellm-dashboard/src/components/mcp_tools/create_mcp_server.test.tsx index 02374a3ffa4..0a3128c6e7f 100644 --- a/ui/litellm-dashboard/src/components/mcp_tools/create_mcp_server.test.tsx +++ b/ui/litellm-dashboard/src/components/mcp_tools/create_mcp_server.test.tsx @@ -202,8 +202,8 @@ describe("CreateMCPServer", () => { await waitFor(() => { expect(screen.getByRole("button", { name: "Authorize & Fetch Tools (browser-only)" })).toBeInTheDocument(); }); - expect(screen.getByText("OAuth Client ID (optional, not saved)")).toBeInTheDocument(); - expect(screen.getByText("OAuth Client Secret (optional, not saved)")).toBeInTheDocument(); + expect(screen.getByText("OAuth Client ID (optional, saved)")).toBeInTheDocument(); + expect(screen.getByText("OAuth Client Secret (optional, saved)")).toBeInTheDocument(); }, ); @@ -436,6 +436,73 @@ describe("CreateMCPServer", () => { ); }); + it.each([ + ["true_passthrough", "True Passthrough (no LiteLLM auth)"], + ["oauth_delegate", "OAuth Delegate (client-supplied upstream token)"], + ])( + "persists admin-entered OAuth app credentials on create for %s while the token stays browser-held", + async (_authType, optionLabel) => { + oauthHook.tokenResponse = { access_token: "upstream-tok", token_type: "Bearer" }; + await selectHttpTransport(); + + const user = userEvent.setup({ delay: null }); + await user.type(getServerNameInput(), "CF_App_Server"); + await user.type(screen.getByPlaceholderText("https://your-mcp-server.com"), "https://example.com/mcp"); + + await selectAntOption("Authentication", optionLabel); + + // Admin declares the org's pre-registered upstream app; unlike the browser-authorized + // token, this is config and must survive onto the server row so internal users' + // Tools-page Authorize relays through it (required for non-DCR upstreams like Slack). + await user.type( + screen.getByPlaceholderText("Leave blank to use dynamic client registration"), + "org-app-client-id", + ); + await user.type(screen.getByPlaceholderText("Leave blank for public clients / PKCE"), "org-app-secret"); + + await waitFor(() => expect(oauthHook.onTokenReceived).toBeTruthy()); + await act(async () => { + oauthHook.onTokenReceived!({ access_token: "upstream-tok", token_type: "Bearer" }, undefined); + }); + + const createdServer = { + server_id: "new-cf-app-server", + server_name: "CF_App_Server", + alias: "CF_App_Server", + url: "https://example.com/mcp", + transport: "http", + auth_type: _authType, + created_at: "2024-01-01T00:00:00Z", + created_by: "user-1", + updated_at: "2024-01-01T00:00:00Z", + updated_by: "user-1", + }; + vi.mocked(networking.createMCPServer).mockResolvedValue(createdServer); + + const submitButton = screen.getByRole("button", { name: "Add MCP Server" }); + await act(async () => { + fireEvent.click(submitButton); + }); + + await waitFor(() => expect(networking.createMCPServer).toHaveBeenCalledTimes(1)); + const [, payload] = vi.mocked(networking.createMCPServer).mock.calls[0]; + + // The declared app persists; the browser-authorized token still appears nowhere in the + // payload and no per-user DB credential is written. + expect(payload.credentials).toEqual({ + client_id: "org-app-client-id", + client_secret: "org-app-secret", + }); + expect(JSON.stringify(payload)).not.toContain("upstream-tok"); + expect(networking.storeMCPOAuthUserCredential).not.toHaveBeenCalled(); + expect(setToken).toHaveBeenCalledWith( + "new-cf-app-server", + expect.objectContaining({ access_token: "upstream-tok" }), + undefined, + ); + }, + ); + it("should not show auth value field when None auth type is selected", async () => { await selectHttpTransport(); diff --git a/ui/litellm-dashboard/src/components/mcp_tools/create_mcp_server.tsx b/ui/litellm-dashboard/src/components/mcp_tools/create_mcp_server.tsx index eb48fd02474..79db031008e 100644 --- a/ui/litellm-dashboard/src/components/mcp_tools/create_mcp_server.tsx +++ b/ui/litellm-dashboard/src/components/mcp_tools/create_mcp_server.tsx @@ -60,6 +60,8 @@ const AUTH_TYPES_REQUIRING_CREDENTIALS = [ AUTH_TYPE.OAUTH2, AUTH_TYPE.OAUTH2_TOKEN_EXCHANGE, AUTH_TYPE.AWS_SIGV4, + AUTH_TYPE.TRUE_PASSTHROUGH, + AUTH_TYPE.OAUTH_DELEGATE, ]; const CREATE_OAUTH_UI_STATE_KEY = "litellm-mcp-oauth-create-state"; @@ -209,7 +211,7 @@ const CreateMCPServer: React.FC = ({ // edit form's onTokenReceived early return. setAuthorizedIdentity(getOAuthAuthorizationIdentity(form.getFieldsValue(true))); NotificationsManager.success( - "Token held for this browser session. Tools can now be previewed and configured; nothing will be saved to LiteLLM.", + "Token held for this browser session. Tools can now be previewed and configured; the token is not saved to LiteLLM.", ); return; } diff --git a/ui/litellm-dashboard/src/components/mcp_tools/mcp_server_edit.test.tsx b/ui/litellm-dashboard/src/components/mcp_tools/mcp_server_edit.test.tsx index adb3e161da5..a1bad0307b0 100644 --- a/ui/litellm-dashboard/src/components/mcp_tools/mcp_server_edit.test.tsx +++ b/ui/litellm-dashboard/src/components/mcp_tools/mcp_server_edit.test.tsx @@ -1,6 +1,7 @@ import React from "react"; import { describe, it, expect, vi, beforeEach } from "vitest"; import { render, screen, waitFor, fireEvent, act } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; import MCPServerEdit from "./mcp_server_edit"; import * as networking from "../networking"; import NotificationsManager from "../molecules/notifications_manager"; @@ -1377,6 +1378,51 @@ describe("MCPServerEdit (OAuth token persistence on save)", () => { }, ); + it.each([["true_passthrough"], ["oauth_delegate"]])( + "persists admin-entered OAuth app credentials in the update payload for the %s mode", + async (authType) => { + mockOauth.tokenResponse = { access_token: "cf-tok", expires_in: 1800, token_type: "bearer" }; + vi.mocked(networking.updateMCPServer).mockResolvedValue({ + ...interactiveOAuthServer, + auth_type: authType, + }); + + render( + , + ); + + const user = userEvent.setup({ delay: null }); + await user.type( + screen.getByPlaceholderText("Leave blank to use dynamic client registration"), + "org-app-client-id", + ); + await user.type(screen.getByPlaceholderText("Leave blank for public clients / PKCE"), "org-app-secret"); + + await act(async () => { + fireEvent.click(screen.getAllByRole("button", { name: "Save Changes" })[0]); + }); + + await waitFor(() => expect(networking.updateMCPServer).toHaveBeenCalledTimes(1)); + const [, payload] = vi.mocked(networking.updateMCPServer).mock.calls[0]; + + // The declared app is config and persists onto the row; the browser-held token still never + // reaches the payload or the per-user credential store. + expect(payload.credentials).toMatchObject({ + client_id: "org-app-client-id", + client_secret: "org-app-secret", + }); + expect(JSON.stringify(payload)).not.toContain("cf-tok"); + expect(networking.storeMCPOAuthUserCredential).not.toHaveBeenCalled(); + }, + ); + it("forwards a newly authorized browser-held token for tool loading before the form is saved", async () => { // Regression: fetchTools keyed the browser-held decision off the saved mcpServer.auth_type, so // after switching the form to true_passthrough and authorizing, the fresh token was not sent as diff --git a/ui/litellm-dashboard/src/components/mcp_tools/mcp_server_edit.tsx b/ui/litellm-dashboard/src/components/mcp_tools/mcp_server_edit.tsx index 7446c96c40e..9b2c5af861f 100644 --- a/ui/litellm-dashboard/src/components/mcp_tools/mcp_server_edit.tsx +++ b/ui/litellm-dashboard/src/components/mcp_tools/mcp_server_edit.tsx @@ -56,6 +56,8 @@ const AUTH_TYPES_REQUIRING_CREDENTIALS = [ AUTH_TYPE.OAUTH2, AUTH_TYPE.OAUTH2_TOKEN_EXCHANGE, AUTH_TYPE.AWS_SIGV4, + AUTH_TYPE.TRUE_PASSTHROUGH, + AUTH_TYPE.OAUTH_DELEGATE, ]; export const EDIT_OAUTH_UI_STATE_KEY = "litellm-mcp-oauth-edit-state"; @@ -202,7 +204,7 @@ const MCPServerEdit: React.FC = ({ }; setToken(mcpServer.server_id, browserHeldToken, userID); NotificationsManager.success( - "Token held for this browser session. Tools can now be loaded and configured; nothing was saved to LiteLLM.", + "Token held for this browser session. Tools can now be loaded and configured; the token is not saved to LiteLLM.", ); return; } From 57051d36d6cdf5d136fb152b3bd9e8b5502fad45 Mon Sep 17 00:00:00 2001 From: Tin Date: Fri, 10 Jul 2026 09:39:37 -0700 Subject: [PATCH 02/99] fix(mcp): keep admin-declared app credentials through OAuth invalidation for the client-forwarded modes --- ui/litellm-dashboard/eslint-metrics.json | 2 +- .../mcp_tools/create_mcp_server.test.tsx | 104 ++++++++++++++++++ .../mcp_tools/create_mcp_server.tsx | 9 ++ .../mcp_tools/mcp_server_edit.test.tsx | 54 +++++++++ .../components/mcp_tools/mcp_server_edit.tsx | 9 ++ .../src/components/mcp_tools/types.tsx | 25 +++++ 6 files changed, 202 insertions(+), 1 deletion(-) diff --git a/ui/litellm-dashboard/eslint-metrics.json b/ui/litellm-dashboard/eslint-metrics.json index 2e204c63a48..9431c278387 100644 --- a/ui/litellm-dashboard/eslint-metrics.json +++ b/ui/litellm-dashboard/eslint-metrics.json @@ -1,7 +1,7 @@ { "@typescript-eslint/no-explicit-any": 1978, "complexity": 129, - "local/no-large-inline-object-arg": 512, + "local/no-large-inline-object-arg": 514, "local/no-long-condition-chain": 233, "max-depth": 59, "no-console": 15 diff --git a/ui/litellm-dashboard/src/components/mcp_tools/create_mcp_server.test.tsx b/ui/litellm-dashboard/src/components/mcp_tools/create_mcp_server.test.tsx index 0a3128c6e7f..cfd0ec1dfb4 100644 --- a/ui/litellm-dashboard/src/components/mcp_tools/create_mcp_server.test.tsx +++ b/ui/litellm-dashboard/src/components/mcp_tools/create_mcp_server.test.tsx @@ -503,6 +503,110 @@ describe("CreateMCPServer", () => { }, ); + it("preserves admin-entered app credentials when the URL changes after authorize for true_passthrough", async () => { + await selectHttpTransport(); + + const user = userEvent.setup({ delay: null }); + await user.type(getServerNameInput(), "CF_Keep_Server"); + await user.type(screen.getByPlaceholderText("https://your-mcp-server.com"), "https://example.com/mcp"); + + await selectAntOption("Authentication", "True Passthrough (no LiteLLM auth)"); + + await user.type( + screen.getByPlaceholderText("Leave blank to use dynamic client registration"), + "org-app-client-id", + ); + await user.type(screen.getByPlaceholderText("Leave blank for public clients / PKCE"), "org-app-secret"); + + await waitFor(() => expect(oauthHook.onTokenReceived).toBeTruthy()); + await act(async () => { + oauthHook.onTokenReceived!({ access_token: "upstream-tok", token_type: "Bearer" }, undefined); + }); + + // Editing the URL after authorize invalidates the held token (identity change), but the + // declared app is config, not minted material: it must survive the invalidation instead of + // being silently reset, or the server would persist without the configured app. + await act(async () => { + fireEvent.change(screen.getByPlaceholderText("https://your-mcp-server.com"), { + target: { value: "https://other.example.com/mcp" }, + }); + }); + + vi.mocked(networking.createMCPServer).mockResolvedValue({ + server_id: "kept-app-server", + server_name: "CF_Keep_Server", + alias: "CF_Keep_Server", + url: "https://other.example.com/mcp", + transport: "http", + auth_type: "true_passthrough", + created_at: "2024-01-01T00:00:00Z", + created_by: "user-1", + updated_at: "2024-01-01T00:00:00Z", + updated_by: "user-1", + }); + + await act(async () => { + fireEvent.click(screen.getByRole("button", { name: "Add MCP Server" })); + }); + + await waitFor(() => expect(networking.createMCPServer).toHaveBeenCalledTimes(1)); + const [, payload] = vi.mocked(networking.createMCPServer).mock.calls[0]; + expect(payload.url).toBe("https://other.example.com/mcp"); + expect(payload.credentials).toEqual({ + client_id: "org-app-client-id", + client_secret: "org-app-secret", + }); + expect(JSON.stringify(payload)).not.toContain("upstream-tok"); + }); + + it("wipes oauth2-minted credentials when the auth type switches to a client-forwarded mode", async () => { + await selectHttpTransport(); + + const user = userEvent.setup({ delay: null }); + await user.type(getServerNameInput(), "Switch_Server"); + await user.type(screen.getByPlaceholderText("https://your-mcp-server.com"), "https://example.com/mcp"); + + await selectAntOption("Authentication", "OAuth"); + + // The oauth2 onTokenReceived branch writes the fetched token AND the DCR client into + // form.credentials; both are minted for the oauth2 identity. + await waitFor(() => expect(oauthHook.onTokenReceived).toBeTruthy()); + await act(async () => { + oauthHook.onTokenReceived!( + { access_token: "oauth2-minted-tok", refresh_token: "oauth2-minted-refresh", token_type: "Bearer" }, + { clientId: "dcr-minted-client", clientSecret: "dcr-minted-secret" }, + ); + }); + + // Switching into a client-forwarded mode changes the identity with auth_type in the changed + // values, so the preserve carve-out must NOT apply: the minted material would otherwise ride + // into a mode that now persists credentials onto the server row. + await selectAntOption("Authentication", "True Passthrough (no LiteLLM auth)"); + + vi.mocked(networking.createMCPServer).mockResolvedValue({ + server_id: "switched-server", + server_name: "Switch_Server", + alias: "Switch_Server", + url: "https://example.com/mcp", + transport: "http", + auth_type: "true_passthrough", + created_at: "2024-01-01T00:00:00Z", + created_by: "user-1", + updated_at: "2024-01-01T00:00:00Z", + updated_by: "user-1", + }); + + await act(async () => { + fireEvent.click(screen.getByRole("button", { name: "Add MCP Server" })); + }); + + await waitFor(() => expect(networking.createMCPServer).toHaveBeenCalledTimes(1)); + const [, payload] = vi.mocked(networking.createMCPServer).mock.calls[0]; + expect(payload.credentials).toBeUndefined(); + expect(JSON.stringify(payload)).not.toContain("dcr-minted-client"); + expect(JSON.stringify(payload)).not.toContain("oauth2-minted-tok"); + }); + it("should not show auth value field when None auth type is selected", async () => { await selectHttpTransport(); diff --git a/ui/litellm-dashboard/src/components/mcp_tools/create_mcp_server.tsx b/ui/litellm-dashboard/src/components/mcp_tools/create_mcp_server.tsx index 79db031008e..a526bc93546 100644 --- a/ui/litellm-dashboard/src/components/mcp_tools/create_mcp_server.tsx +++ b/ui/litellm-dashboard/src/components/mcp_tools/create_mcp_server.tsx @@ -18,6 +18,7 @@ import { getOAuthAuthorizationIdentity, CLEARED_ON_INVALIDATION, isHeldOAuthTokenStale, + preservedDeclaredAppCredentials, } from "./types"; import OAuthFormFields from "./OAuthFormFields"; import TruePassthroughWarning from "./TruePassthroughWarning"; @@ -248,7 +249,15 @@ const CreateMCPServer: React.FC = ({ clearTools(); resetOAuthFlow(); setAuthorizedIdentity(undefined); + const keptAppCredentials = preservedDeclaredAppCredentials( + form.getFieldValue("auth_type"), + "auth_type" in changedValues, + form.getFieldValue("credentials"), + ); form.resetFields([...CLEARED_ON_INVALIDATION]); + if (keptAppCredentials) { + form.setFieldsValue({ credentials: keptAppCredentials }); + } const preserved = Object.fromEntries( CLEARED_ON_INVALIDATION.filter((key) => key in changedValues).map((key) => [key, changedValues[key]]), ); diff --git a/ui/litellm-dashboard/src/components/mcp_tools/mcp_server_edit.test.tsx b/ui/litellm-dashboard/src/components/mcp_tools/mcp_server_edit.test.tsx index a1bad0307b0..4e0129d4ba1 100644 --- a/ui/litellm-dashboard/src/components/mcp_tools/mcp_server_edit.test.tsx +++ b/ui/litellm-dashboard/src/components/mcp_tools/mcp_server_edit.test.tsx @@ -1423,6 +1423,60 @@ describe("MCPServerEdit (OAuth token persistence on save)", () => { }, ); + it.each([["true_passthrough"], ["oauth_delegate"]])( + "preserves admin-entered app credentials when the URL changes after authorize for the %s mode", + async (authType) => { + vi.mocked(networking.updateMCPServer).mockResolvedValue({ + ...interactiveOAuthServer, + auth_type: authType, + }); + + render( + , + ); + + const user = userEvent.setup({ delay: null }); + await user.type( + screen.getByPlaceholderText("Leave blank to use dynamic client registration"), + "org-app-client-id", + ); + await user.type(screen.getByPlaceholderText("Leave blank for public clients / PKCE"), "org-app-secret"); + + act(() => { + mockOauth.onTokenReceived?.({ access_token: "cf-tok", token_type: "bearer" }); + }); + + // The URL edit invalidates the held browser token (removeToken fires), but the declared app + // is config and must survive the invalidation into the update payload. + await act(async () => { + fireEvent.change(screen.getByPlaceholderText("https://your-mcp-server.com"), { + target: { value: "https://other.example.com/mcp" }, + }); + }); + expect(mockRemoveToken).toHaveBeenCalledWith("oauth_server_1", "user-1"); + + await act(async () => { + fireEvent.click(screen.getAllByRole("button", { name: "Save Changes" })[0]); + }); + + await waitFor(() => expect(networking.updateMCPServer).toHaveBeenCalledTimes(1)); + const [, payload] = vi.mocked(networking.updateMCPServer).mock.calls[0]; + expect(payload.url).toBe("https://other.example.com/mcp"); + expect(payload.credentials).toMatchObject({ + client_id: "org-app-client-id", + client_secret: "org-app-secret", + }); + expect(JSON.stringify(payload)).not.toContain("cf-tok"); + }, + ); + it("forwards a newly authorized browser-held token for tool loading before the form is saved", async () => { // Regression: fetchTools keyed the browser-held decision off the saved mcpServer.auth_type, so // after switching the form to true_passthrough and authorizing, the fresh token was not sent as diff --git a/ui/litellm-dashboard/src/components/mcp_tools/mcp_server_edit.tsx b/ui/litellm-dashboard/src/components/mcp_tools/mcp_server_edit.tsx index 9b2c5af861f..62f9b7e6931 100644 --- a/ui/litellm-dashboard/src/components/mcp_tools/mcp_server_edit.tsx +++ b/ui/litellm-dashboard/src/components/mcp_tools/mcp_server_edit.tsx @@ -8,6 +8,7 @@ import { getOAuthAuthorizationIdentity, CLEARED_ON_INVALIDATION, isHeldOAuthTokenStale, + preservedDeclaredAppCredentials, OAUTH_FLOW, MCP_OAUTH2_FLOW_M2M, MCP_OAUTH2_FLOW_INTERACTIVE, @@ -409,7 +410,15 @@ const MCPServerEdit: React.FC = ({ } setTools([]); resetOAuthFlow(); + const keptAppCredentials = preservedDeclaredAppCredentials( + getEffectiveAuthType(), + "auth_type" in changedValues, + form.getFieldValue("credentials"), + ); form.resetFields([...CLEARED_ON_INVALIDATION]); + if (keptAppCredentials) { + form.setFieldsValue({ credentials: keptAppCredentials }); + } const preserved = Object.fromEntries( CLEARED_ON_INVALIDATION.filter((key) => key in changedValues).map((key) => [key, changedValues[key]]), ); diff --git a/ui/litellm-dashboard/src/components/mcp_tools/types.tsx b/ui/litellm-dashboard/src/components/mcp_tools/types.tsx index 3eba8b30968..e49fd5d57e4 100644 --- a/ui/litellm-dashboard/src/components/mcp_tools/types.tsx +++ b/ui/litellm-dashboard/src/components/mcp_tools/types.tsx @@ -96,6 +96,31 @@ export const getOAuthAuthorizationIdentity = (values: Record): // edit forms so what gets wiped cannot drift. export const CLEARED_ON_INVALIDATION = ["credentials"] as const; +// The carve-out to the wipe above for the client-forwarded token modes: their onTokenReceived branch +// never writes minted material into form.credentials, so for them the field only ever holds the +// admin-DECLARED upstream app (persisted as server config since the modes joined +// AUTH_TYPES_REQUIRING_CREDENTIALS), and an intra-mode identity change (e.g. a URL edit after +// Authorize) must not silently discard it. Two guards make the preserve safe: it never applies when +// auth_type itself changed (the previous mode's onTokenReceived may have written a fetched token or +// DCR client into the same field, and those are minted for the old mode), and it only ever keeps the +// declared-app keys, so token-shaped keys can never ride through a preserve. Shared by the create and +// edit forms so the carve-out cannot drift. +const DECLARED_APP_CREDENTIAL_KEYS = ["client_id", "client_secret"] as const; + +export const preservedDeclaredAppCredentials = ( + authType: string | null | undefined, + authTypeChanged: boolean, + credentials: Record | null | undefined, +): Record | undefined => { + if (!isClientForwardedTokenMode(authType) || authTypeChanged || !credentials) return undefined; + const kept = Object.fromEntries( + DECLARED_APP_CREDENTIAL_KEYS.filter((key) => typeof credentials[key] === "string" && credentials[key] !== "").map( + (key) => [key, credentials[key] as string], + ), + ); + return Object.keys(kept).length > 0 ? kept : undefined; +}; + // True when a token was authorized in this session (authorizedIdentity recorded at mint time) and the // form's current identity no longer matches it. Every invalidation decision in both forms goes through // this single check: onValuesChange for user edits, and an explicit recheck after any programmatic From cf1b407fbe0941890eb309e36d8c12f4ccb3b42d Mon Sep 17 00:00:00 2001 From: Tin Date: Fri, 10 Jul 2026 09:51:10 -0700 Subject: [PATCH 03/99] fix(mcp): state the keep-existing convention for blank client fields and add explicit app removal on edit --- .../mcp_tools/PassthroughAuthorizeSection.tsx | 29 +++++++++++++- .../mcp_tools/mcp_server_edit.test.tsx | 40 ++++++++++++++++++- .../components/mcp_tools/mcp_server_edit.tsx | 12 ++++++ 3 files changed, 77 insertions(+), 4 deletions(-) diff --git a/ui/litellm-dashboard/src/components/mcp_tools/PassthroughAuthorizeSection.tsx b/ui/litellm-dashboard/src/components/mcp_tools/PassthroughAuthorizeSection.tsx index 314b45fff6c..8d53e8b86ee 100644 --- a/ui/litellm-dashboard/src/components/mcp_tools/PassthroughAuthorizeSection.tsx +++ b/ui/litellm-dashboard/src/components/mcp_tools/PassthroughAuthorizeSection.tsx @@ -1,5 +1,5 @@ import React from "react"; -import { Button, Form, Input } from "antd"; +import { Button, Checkbox, Form, Input } from "antd"; import { isClientForwardedTokenMode } from "./types"; interface PassthroughOAuthFlow { @@ -19,13 +19,26 @@ interface PassthroughOAuthFlow { * pre-registered Slack app); unlike the token they ARE saved onto the server * as declared config, so internal users' Authorize relays through the org's * app instead of dead-ending on upstreams that cannot mint clients. + * + * Blank fields follow the same convention as the M2M credential fields: on + * create they mean "no app configured" (dynamic client registration), while on + * edit the backend's partial update keeps whatever app is already stored, so + * blanks mean "keep existing". Removing a stored app is therefore an explicit + * action (the checkbox below, edit only), which saves an explicit-null + * credential write instead of omitting the field. */ export default function PassthroughAuthorizeSection({ authType, oauthFlow, + isEditing = false, + removeStoredApp = false, + onRemoveStoredAppChange, }: { authType?: string | null; oauthFlow: PassthroughOAuthFlow; + isEditing?: boolean; + removeStoredApp?: boolean; + onRemoveStoredAppChange?: (remove: boolean) => void; }) { if (!isClientForwardedTokenMode(authType)) return null; const authorizeButtonLabels: Record = { @@ -33,6 +46,9 @@ export default function PassthroughAuthorizeSection({ exchanging: "Exchanging authorization code...", }; const authorizeButtonLabel = authorizeButtonLabels[oauthFlow.status] ?? "Authorize & Fetch Tools (browser-only)"; + const blankMeaning = isEditing + ? "Leave blank to keep the currently saved app (if any)" + : "Leave blank to use dynamic client registration"; return (

@@ -47,7 +63,8 @@ export default function PassthroughAuthorizeSection({ extra="Set this to make everyone authorize through a specific app; required for upstreams without dynamic client registration (e.g. a pre-registered Slack app)." > @@ -57,9 +74,17 @@ export default function PassthroughAuthorizeSection({ > + {isEditing && onRemoveStoredAppChange && ( + onRemoveStoredAppChange(e.target.checked)}> + + Remove the saved OAuth app on save (the server goes back to dynamic client registration) + + + )}

= ({ accessToken }) => { } paginationMode="client" pageSizeOptions={[50, 100]} + filterMode="client" + columnFilters={columnFilters} + onColumnFiltersChange={setColumnFilters} + globalFilter={globalFilter} + onGlobalFilterChange={setGlobalFilter} onRowClick={fetchRunDetail} size="compact" + toolbar={(table) => ( + <> + setFiltersOpen(true)} + /> + + {({ get, set }) => ( + <> + + + + + set("workflow_type", event.target.value)} + placeholder="Filter by type…" + /> + + + )} + + + )} /> {/* detail drawer */} diff --git a/ui/litellm-dashboard/src/components/shared/DataTable/DataTable.test.tsx b/ui/litellm-dashboard/src/components/shared/DataTable/DataTable.test.tsx index df42c156975..c32dbe10455 100644 --- a/ui/litellm-dashboard/src/components/shared/DataTable/DataTable.test.tsx +++ b/ui/litellm-dashboard/src/components/shared/DataTable/DataTable.test.tsx @@ -29,6 +29,16 @@ const nameCellColumns: ColumnDef[] = [ }, ]; +const filterableColumns: ColumnDef[] = [ + { + accessorKey: "name", + header: "Name", + meta: { title: "Name" }, + filterFn: (row, columnId, value) => row.getValue(columnId) === value, + cell: ({ row }) => {row.original.name}, + }, +]; + const headerCycleColumns: ColumnDef[] = [ { accessorKey: "name", @@ -195,10 +205,93 @@ describe("DataTable pagination", () => { }); }); +describe("DataTable filtering", () => { + it("client mode filters rows by columnFilters", () => { + const { rerender } = render( + , + ); + expect(names()).toEqual(["Charlie", "Alice", "Bob"]); + + rerender( + , + ); + expect(names()).toEqual(["Alice"]); + }); + + it("client global filter matches substrings across columns", () => { + const { rerender } = render( + , + ); + expect(names()).toEqual(["Charlie", "Alice", "Bob"]); + + rerender( + , + ); + expect(names()).toEqual(["Alice"]); + }); + + it("server mode never filters locally even when columnFilters is set", () => { + render( + , + ); + expect(names()).toEqual(["Charlie", "Alice", "Bob"]); + }); + + it("throws when server filtering is missing required props", () => { + const spy = vi.spyOn(console, "error").mockImplementation(() => {}); + expect(() => render()).toThrow( + /filterMode='server'/, + ); + spy.mockRestore(); + }); +}); + +describe("DataTable loading", () => { + it("renders skeleton rows while loading and real rows once loaded", () => { + const { rerender } = render(); + expect(screen.getAllByTestId("skeleton-row").length).toBeGreaterThan(0); + expect(screen.queryByTestId("name-cell")).toBeNull(); + + rerender(); + expect(screen.queryAllByTestId("skeleton-row")).toHaveLength(0); + expect(names()).toEqual(["Charlie", "Alice", "Bob"]); + }); +}); + describe("DataTable column visibility", () => { it("hides a column when toggled off in the view-options menu", async () => { const user = userEvent.setup(); - render( + const { container } = render( { />, ); - expect(screen.getByText("Email")).toBeInTheDocument(); + expect(container.querySelector('th[data-header-id="email"]')).not.toBeNull(); await user.click(screen.getByTestId("view-options-trigger")); await user.click(await screen.findByTestId("view-option-email")); - await waitFor(() => expect(screen.queryByText("Email")).not.toBeInTheDocument()); + await waitFor(() => expect(container.querySelector('th[data-header-id="email"]')).toBeNull()); await user.click(screen.getByTestId("view-option-email")); - await waitFor(() => expect(screen.getByText("Email")).toBeInTheDocument()); + await waitFor(() => expect(container.querySelector('th[data-header-id="email"]')).not.toBeNull()); }); it("omits columns that opt out of hiding from the menu", async () => { diff --git a/ui/litellm-dashboard/src/components/shared/DataTable/DataTable.tsx b/ui/litellm-dashboard/src/components/shared/DataTable/DataTable.tsx index 2e95ee170fa..dcc341f3c78 100644 --- a/ui/litellm-dashboard/src/components/shared/DataTable/DataTable.tsx +++ b/ui/litellm-dashboard/src/components/shared/DataTable/DataTable.tsx @@ -4,12 +4,14 @@ import { type Cell, type Column, type ColumnDef, + type ColumnFiltersState, type ColumnPinningState, type ColumnSizingState, type ExpandedState, flexRender, getCoreRowModel, getExpandedRowModel, + getFilteredRowModel, getPaginationRowModel, getSortedRowModel, type Header, @@ -21,9 +23,11 @@ import { useReactTable, type VisibilityState, } from "@tanstack/react-table"; +import { SearchX } from "lucide-react"; import * as React from "react"; import { Fragment, useState } from "react"; +import { Skeleton } from "@/components/ui/skeleton"; import { Table as TableRoot, TableBody, @@ -37,7 +41,7 @@ import { cn } from "@/lib/cva.config"; import "./columnMeta"; import { DataTablePagination, DEFAULT_PAGE_SIZE_OPTIONS } from "./DataTablePagination"; -import type { ColumnPinnedSide, DataTableProps, DataTableSize, PaginationMode, SortingMode } from "./types"; +import type { ColumnPinnedSide, DataTableProps, DataTableSize, FilterMode, PaginationMode, SortingMode } from "./types"; const INTERACTIVE_SELECTOR = "button, a, input, select, textarea, [role=checkbox], [data-row-click-exempt]"; @@ -60,14 +64,22 @@ export function validateDataTableConfig( props.pagination === undefined || props.onPaginationChange === undefined || props.rowCount === undefined; const serverPaginationIncomplete = props.paginationMode === "server" && serverPaginationPropsMissing; + const serverFilteringIncomplete = + props.filterMode === "server" && (props.columnFilters === undefined || props.onColumnFiltersChange === undefined); + const bothSortingSources = props.defaultSorting !== undefined && props.sorting !== undefined; + const bothFilterSources = props.defaultColumnFilters !== undefined && props.columnFilters !== undefined; return [ serverSortingIncomplete ? "sortingMode='server' requires both `sorting` and `onSortingChange`." : null, serverPaginationIncomplete ? "paginationMode='server' requires `pagination`, `onPaginationChange`, and `rowCount`." : null, + serverFilteringIncomplete ? "filterMode='server' requires both `columnFilters` and `onColumnFiltersChange`." : null, bothSortingSources ? "Provide either `defaultSorting` (uncontrolled) or `sorting` (controlled), not both." : null, + bothFilterSources + ? "Provide either `defaultColumnFilters` (uncontrolled) or `columnFilters` (controlled), not both." + : null, ].filter((message): message is string => message !== null); } @@ -93,9 +105,11 @@ function derivePinning(columns: ColumnDef[]): Colu function buildRowModels( sortingMode: SortingMode, paginationMode: PaginationMode, + filterMode: FilterMode, getRowCanExpand: ((row: Row) => boolean) | undefined, ): Partial> { return { + ...(filterMode === "client" ? { getFilteredRowModel: getFilteredRowModel() } : {}), ...(sortingMode === "client" ? { getSortedRowModel: getSortedRowModel() } : {}), ...(paginationMode === "client" ? { getPaginationRowModel: getPaginationRowModel() } : {}), ...(getRowCanExpand !== undefined ? { getRowCanExpand, getExpandedRowModel: getExpandedRowModel() } : {}), @@ -307,6 +321,49 @@ function MessageRow({ colSpan, children }: { colSpan: number; children: React.Re ); } +function DefaultEmptyState() { + return ( +
+
+ +
+
No results
+
No rows match your search or filters.
+
+ ); +} + +function SkeletonRows({ + rowCount, + columnCount, + size, + message, +}: { + rowCount: number; + columnCount: number; + size: DataTableSize; + message?: string; +}) { + const rowKeys = Array.from({ length: Math.max(rowCount, 1) }, (_, index) => index); + const columnKeys = Array.from({ length: Math.max(columnCount, 1) }, (_, index) => index); + return ( + + {rowKeys.map((rowKey) => ( + + {columnKeys.map((columnKey) => ( + + + {rowKey === 0 && columnKey === 0 && message !== undefined ? ( + {message} + ) : null} + + ))} + + ))} + + ); +} + function useControllable( controlled: T | undefined, controlledOnChange: OnChangeFn | undefined, @@ -334,6 +391,12 @@ function useDataTableInstance(props: DataTablePro onPaginationChange, rowCount, pageSizeOptions = DEFAULT_PAGE_SIZE_OPTIONS, + filterMode = "none", + columnFilters, + onColumnFiltersChange, + defaultColumnFilters, + globalFilter, + onGlobalFilterChange, enableColumnResizing = false, columnResizeMode = "onEnd", defaultColumnVisibility, @@ -348,6 +411,12 @@ function useDataTableInstance(props: DataTablePro pageIndex: 0, pageSize: pageSizeOptions[0] ?? 25, }); + const filterState = useControllable( + columnFilters, + onColumnFiltersChange, + defaultColumnFilters ?? [], + ); + const globalFilterState = useControllable(globalFilter, onGlobalFilterChange, ""); const expandedState = useControllable(expanded, onExpandedChange, {}); const [columnVisibility, setColumnVisibility] = useState(defaultColumnVisibility ?? {}); const [columnSizing, setColumnSizing] = useState({}); @@ -360,6 +429,8 @@ function useDataTableInstance(props: DataTablePro state: { sorting: sortingState.value, pagination: paginationState.value, + columnFilters: filterState.value, + globalFilter: globalFilterState.value, expanded: expandedState.value, columnVisibility, columnSizing, @@ -367,16 +438,19 @@ function useDataTableInstance(props: DataTablePro initialState: { columnPinning }, manualSorting: sortingMode === "server", manualPagination: paginationMode === "server", + manualFiltering: filterMode === "server", enableSortingRemoval, enableColumnResizing, columnResizeMode, onSortingChange: sortingState.onChange, onPaginationChange: paginationState.onChange, + onColumnFiltersChange: filterState.onChange, + onGlobalFilterChange: globalFilterState.onChange, onExpandedChange: expandedState.onChange, onColumnVisibilityChange: setColumnVisibility, onColumnSizingChange: setColumnSizing, getCoreRowModel: getCoreRowModel(), - ...buildRowModels(sortingMode, paginationMode, expansionGuard), + ...buildRowModels(sortingMode, paginationMode, filterMode, expansionGuard), ...(getRowId !== undefined ? { getRowId } : {}), ...(paginationMode === "server" && rowCount !== undefined ? { rowCount } : {}), }; @@ -397,7 +471,8 @@ export function DataTable(props: DataTableProps(props: DataTableProps { if (isLoading) { - return {loadingMessage}; + return ( + + ); } if (rows.length === 0) { - return {noDataMessage}; + return {noDataMessage ?? }; } return rows.map((row) => ( (props: DataTableProps - {toolbar !== undefined &&
{toolbar(table)}
} -
- - - {table.getHeaderGroups().map((headerGroup) => ( - - {headerGroup.headers.map((header) => ( - - ))} - - ))} - - {renderBody()} - {footer !== undefined && {footer(table)}} - +
+ {toolbar !== undefined &&
{toolbar(table)}
} +
+ + + {table.getHeaderGroups().map((headerGroup) => ( + + {headerGroup.headers.map((header) => ( + + ))} + + ))} + + {renderBody()} + {footer !== undefined && {footer(table)}} + +
+ {paginationNode !== null &&
{paginationNode}
}
- {renderPagination()}
); } diff --git a/ui/litellm-dashboard/src/components/shared/DataTable/DataTableFilterDrawer.test.tsx b/ui/litellm-dashboard/src/components/shared/DataTable/DataTableFilterDrawer.test.tsx new file mode 100644 index 00000000000..0770c17cba6 --- /dev/null +++ b/ui/litellm-dashboard/src/components/shared/DataTable/DataTableFilterDrawer.test.tsx @@ -0,0 +1,98 @@ +import type { ColumnDef, ColumnFiltersState } from "@tanstack/react-table"; +import { render, screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { useState } from "react"; +import { describe, expect, it } from "vitest"; + +import { DataTable } from "./DataTable"; +import { DataTableFilterDrawer } from "./DataTableFilterDrawer"; +import { DataTableToolbar } from "./DataTableToolbar"; + +interface Person { + id: string; + name: string; +} + +const DATA: Person[] = [ + { id: "a", name: "Alice" }, + { id: "b", name: "Bob" }, + { id: "c", name: "Carol" }, +]; + +const columns: ColumnDef[] = [ + { + accessorKey: "name", + header: "Name", + meta: { title: "Name" }, + filterFn: (row, columnId, value) => row.getValue(columnId) === value, + cell: ({ row }) => {row.original.name}, + }, +]; + +const names = (): (string | null)[] => screen.getAllByTestId("name-cell").map((el) => el.textContent); + +function Harness({ initialFilters }: { initialFilters?: ColumnFiltersState }) { + const [open, setOpen] = useState(false); + return ( + ( + <> + setOpen(true)} /> + + {({ get, set }) => ( + set("name", event.target.value)} + /> + )} + + + )} + /> + ); +} + +describe("DataTableFilterDrawer", () => { + it("stages edits and only commits them to the table on Apply", async () => { + const user = userEvent.setup(); + render(); + expect(names()).toEqual(["Alice", "Bob", "Carol"]); + + await user.click(screen.getByTestId("datatable-filters-trigger")); + await user.type(await screen.findByTestId("draft-name"), "Bob"); + + expect(names()).toEqual(["Alice", "Bob", "Carol"]); + expect(screen.queryByTestId("filter-chip-name")).toBeNull(); + + await user.click(screen.getByTestId("filter-drawer-apply")); + expect(names()).toEqual(["Bob"]); + expect(screen.getByTestId("filter-chip-name")).toHaveTextContent("Bob"); + }); + + it("seeds the draft from committed filters when opened", async () => { + const user = userEvent.setup(); + render(); + expect(names()).toEqual(["Bob"]); + + await user.click(screen.getByTestId("datatable-filters-trigger")); + expect(await screen.findByTestId("draft-name")).toHaveValue("Bob"); + }); + + it("reset clears the committed filters and the draft", async () => { + const user = userEvent.setup(); + render(); + + await user.click(screen.getByTestId("datatable-filters-trigger")); + await user.click(await screen.findByTestId("filter-drawer-reset")); + + expect(names()).toEqual(["Alice", "Bob", "Carol"]); + expect(screen.queryByTestId("filter-chip-name")).toBeNull(); + expect(screen.getByTestId("draft-name")).toHaveValue(""); + }); +}); diff --git a/ui/litellm-dashboard/src/components/shared/DataTable/DataTableFilterDrawer.tsx b/ui/litellm-dashboard/src/components/shared/DataTable/DataTableFilterDrawer.tsx new file mode 100644 index 00000000000..5b7460eb62c --- /dev/null +++ b/ui/litellm-dashboard/src/components/shared/DataTable/DataTableFilterDrawer.tsx @@ -0,0 +1,107 @@ +"use client"; + +import type { ColumnFiltersState, Table } from "@tanstack/react-table"; +import * as React from "react"; + +import { Button } from "@/components/ui/button"; +import { Sheet, SheetContent, SheetDescription, SheetFooter, SheetHeader, SheetTitle } from "@/components/ui/sheet"; + +export interface FilterDraft { + get: (columnId: string) => unknown; + set: (columnId: string, value: unknown) => void; +} + +interface DataTableFilterDrawerProps { + table: Table; + open: boolean; + onOpenChange: (open: boolean) => void; + title?: string; + description?: React.ReactNode; + applyLabel?: string; + resetLabel?: string; + children: (draft: FilterDraft) => React.ReactNode; +} + +function isEmpty(value: unknown): boolean { + if (Array.isArray(value)) { + return value.length === 0; + } + return value === undefined || value === null || value === ""; +} + +function toDraft(filters: ColumnFiltersState): Record { + return Object.fromEntries(filters.map((filter) => [filter.id, filter.value])); +} + +function toFilters(draft: Record): ColumnFiltersState { + return Object.entries(draft) + .filter(([, value]) => !isEmpty(value)) + .map(([id, value]) => ({ id, value })); +} + +export function DataTableFilterDrawer({ + table, + open, + onOpenChange, + title = "Filters", + description, + applyLabel = "Apply Filters", + resetLabel = "Reset", + children, +}: DataTableFilterDrawerProps) { + const [draft, setDraft] = React.useState>(() => toDraft(table.getState().columnFilters)); + const [wasOpen, setWasOpen] = React.useState(open); + + if (open !== wasOpen) { + setWasOpen(open); + if (open) { + setDraft(toDraft(table.getState().columnFilters)); + } + } + + const helpers: FilterDraft = { + get: (columnId) => draft[columnId], + set: (columnId, value) => setDraft((previous) => ({ ...previous, [columnId]: value })), + }; + + const apply = () => { + table.setColumnFilters(toFilters(draft)); + onOpenChange(false); + }; + + const reset = () => { + setDraft({}); + table.setColumnFilters([]); + }; + + return ( + + + + {title} + {description !== undefined && {description}} + +
+ {children(helpers)} +
+ + + + + + + ); +} + +export function DataTableFilterField({ label, children }: { label: string; children: React.ReactNode }) { + return ( +
+ + {children} +
+ ); +} diff --git a/ui/litellm-dashboard/src/components/shared/DataTable/DataTablePagination.tsx b/ui/litellm-dashboard/src/components/shared/DataTable/DataTablePagination.tsx index 5a30b12f27f..7802465ef74 100644 --- a/ui/litellm-dashboard/src/components/shared/DataTable/DataTablePagination.tsx +++ b/ui/litellm-dashboard/src/components/shared/DataTable/DataTablePagination.tsx @@ -37,7 +37,7 @@ export function DataTablePagination({ const lastPage = Math.max(pageCount - 1, 0); return ( -
+
Rows per page onSearchChange(event.target.value)} + placeholder={searchPlaceholder} + className="h-8 w-56 pl-8" + data-testid="datatable-search" + /> +
)} - {onToggleFilters !== undefined && ( - + {filters.map((filter) => ( + + {labelFor(filter.id)}: + {valueFor(filter.id, filter.value)} + + + ))} + {filters.length > 0 && ( + + )} +
+
+ {children} + {onRefresh !== undefined && ( + + )} + {showViewOptions && } + {onOpenFilters !== undefined && ( + )} - {showReset && }
- {children !== undefined &&
{children}
}
); } diff --git a/ui/litellm-dashboard/src/components/shared/DataTable/DataTableViewOptions.tsx b/ui/litellm-dashboard/src/components/shared/DataTable/DataTableViewOptions.tsx index ab56aafe7b5..f462481ef9c 100644 --- a/ui/litellm-dashboard/src/components/shared/DataTable/DataTableViewOptions.tsx +++ b/ui/litellm-dashboard/src/components/shared/DataTable/DataTableViewOptions.tsx @@ -2,7 +2,7 @@ import { Menu } from "@base-ui/react/menu"; import type { Table } from "@tanstack/react-table"; -import { Check, SlidersHorizontal } from "lucide-react"; +import { Check, Columns3 } from "lucide-react"; import { Button } from "@/components/ui/button"; @@ -24,7 +24,7 @@ export function DataTableViewOptions({ table, label = "View", className } - + {label} } @@ -44,7 +44,8 @@ export function DataTableViewOptions({ table, label = "View", className } - {column.columnDef.meta?.title ?? column.id} + {column.columnDef.meta?.title ?? + (typeof column.columnDef.header === "string" ? column.columnDef.header : column.id)} ))} diff --git a/ui/litellm-dashboard/src/components/shared/DataTable/index.ts b/ui/litellm-dashboard/src/components/shared/DataTable/index.ts index 49a4430bbee..c4218f6051a 100644 --- a/ui/litellm-dashboard/src/components/shared/DataTable/index.ts +++ b/ui/litellm-dashboard/src/components/shared/DataTable/index.ts @@ -1,6 +1,7 @@ import "./columnMeta"; export { DataTable, DataTableConfigError, validateDataTableConfig } from "./DataTable"; +export { DataTableFilterDrawer, DataTableFilterField, type FilterDraft } from "./DataTableFilterDrawer"; export { DataTablePagination, DEFAULT_PAGE_SIZE_OPTIONS } from "./DataTablePagination"; export { DataTableToolbar } from "./DataTableToolbar"; export { DataTableViewOptions } from "./DataTableViewOptions"; @@ -11,6 +12,7 @@ export type { ColumnResizeMode, DataTableProps, DataTableSize, + FilterMode, PaginationMode, SortingMode, } from "./types"; diff --git a/ui/litellm-dashboard/src/components/shared/DataTable/types.ts b/ui/litellm-dashboard/src/components/shared/DataTable/types.ts index 8fa6f21c4d3..045cfa895e1 100644 --- a/ui/litellm-dashboard/src/components/shared/DataTable/types.ts +++ b/ui/litellm-dashboard/src/components/shared/DataTable/types.ts @@ -1,5 +1,6 @@ import type { ColumnDef, + ColumnFiltersState, ExpandedState, OnChangeFn, PaginationState, @@ -13,6 +14,7 @@ import type * as React from "react"; export type SortingMode = "none" | "client" | "server"; export type PaginationMode = "none" | "client" | "server"; +export type FilterMode = "none" | "client" | "server"; export type ColumnResizeMode = "onEnd" | "onChange"; export type DataTableSize = "compact" | "default"; export type ColumnPinnedSide = "left" | "right"; @@ -24,6 +26,7 @@ export interface DataTableProps { isLoading?: boolean; loadingMessage?: string; + skeletonRowCount?: number; noDataMessage?: React.ReactNode; sortingMode?: SortingMode; @@ -38,6 +41,14 @@ export interface DataTableProps { rowCount?: number; pageSizeOptions?: number[]; + filterMode?: FilterMode; + columnFilters?: ColumnFiltersState; + onColumnFiltersChange?: OnChangeFn; + defaultColumnFilters?: ColumnFiltersState; + + globalFilter?: string; + onGlobalFilterChange?: OnChangeFn; + enableColumnResizing?: boolean; columnResizeMode?: ColumnResizeMode; defaultColumnVisibility?: VisibilityState; diff --git a/ui/litellm-dashboard/src/components/team/TeamInfo.test.tsx b/ui/litellm-dashboard/src/components/team/TeamInfo.test.tsx index 4ccfb891417..f0e27607ffb 100644 --- a/ui/litellm-dashboard/src/components/team/TeamInfo.test.tsx +++ b/ui/litellm-dashboard/src/components/team/TeamInfo.test.tsx @@ -583,7 +583,7 @@ describe("TeamInfoView", () => { await waitFor(() => { expect(screen.getByRole("button", { name: "Filters" })).toBeInTheDocument(); }); - expect(screen.getByRole("button", { name: "Reset Filters" })).toBeInTheDocument(); + expect(screen.getByRole("button", { name: "Columns" })).toBeInTheDocument(); expect(screen.getByTestId("pagination-range")).toHaveTextContent("Showing 1-1 of 1"); expect(screen.getByTestId("pagination-prev")).toBeInTheDocument(); expect(screen.getByTestId("pagination-next")).toBeInTheDocument(); diff --git a/ui/litellm-dashboard/src/components/team/TeamVirtualKeysTable.test.tsx b/ui/litellm-dashboard/src/components/team/TeamVirtualKeysTable.test.tsx index fd81aaaad99..ec6e97a1d23 100644 --- a/ui/litellm-dashboard/src/components/team/TeamVirtualKeysTable.test.tsx +++ b/ui/litellm-dashboard/src/components/team/TeamVirtualKeysTable.test.tsx @@ -4,8 +4,6 @@ import { beforeEach, describe, expect, it, vi, MockedFunction } from "vitest"; import { renderWithProviders } from "../../../tests/test-utils"; import { TeamVirtualKeysTable } from "./TeamVirtualKeysTable"; import { KeysResponse, useKeys } from "@/app/(dashboard)/hooks/keys/useKeys"; -import { fetchTeamFilterOptions } from "../key_team_helpers/filter_helpers"; -import useAuthorized from "@/app/(dashboard)/hooks/useAuthorized"; import { KeyResponse } from "../key_team_helpers/key_list"; import { Organization } from "../networking"; @@ -13,18 +11,6 @@ vi.mock("@/app/(dashboard)/hooks/keys/useKeys", () => ({ useKeys: vi.fn(), })); -vi.mock("@/app/(dashboard)/hooks/useAuthorized", () => ({ - default: vi.fn(), -})); - -vi.mock("../key_team_helpers/filter_helpers", () => ({ - fetchTeamFilterOptions: vi.fn().mockResolvedValue({ - keyAliases: [], - organizationIds: [], - userIds: [], - }), -})); - vi.mock("../key_team_helpers/fetch_available_models_team_key", () => ({ getModelDisplayName: vi.fn((model: string) => model), })); @@ -39,7 +25,6 @@ vi.mock("../templates/key_info_view", () => ({ })); const mockUseKeys = useKeys as MockedFunction; -const mockUseAuthorized = useAuthorized as MockedFunction; const createMockKey = (overrides: Partial = {}): KeyResponse => ({ @@ -85,7 +70,6 @@ describe("TeamVirtualKeysTable", () => { beforeEach(() => { vi.clearAllMocks(); - mockUseAuthorized.mockReturnValue({ accessToken: "test-token" } as any); mockUseKeys.mockReturnValue({ data: { keys: [], total_count: 0, current_page: 1, total_pages: 1 } as KeysResponse, isPending: false, @@ -262,30 +246,50 @@ describe("TeamVirtualKeysTable", () => { await waitFor(() => expect(mockUseKeys).toHaveBeenLastCalledWith(1, 50, expect.anything())); }); - it("resets the sort order to the default when filters are reset", async () => { + it("maps the User ID drawer filter to a server-side useKeys query and clears it", async () => { const user = userEvent.setup(); - const result = { + mockUseKeys.mockReturnValue({ data: { keys: [createMockKey()], total_count: 1, current_page: 1, total_pages: 1 }, isPending: false, isFetching: false, refetch: vi.fn(), - } as unknown as ReturnType; - mockUseKeys.mockReturnValue(result); + } as unknown as ReturnType); renderWithProviders(); - await user.click(await screen.findByTestId("sort-header-created_at")); + await user.click(await screen.findByTestId("datatable-filters-trigger")); + const drawerBody = await screen.findByTestId("filter-drawer-body"); + const userInput = drawerBody.querySelector("input") as HTMLElement; + await user.type(userInput, "user-42"); + await user.click(screen.getByTestId("filter-drawer-apply")); + await waitFor(() => - expect(mockUseKeys).toHaveBeenLastCalledWith(1, 50, expect.objectContaining({ sortOrder: "asc" })), + expect(mockUseKeys).toHaveBeenLastCalledWith(1, 50, expect.objectContaining({ userID: "user-42" })), ); - await user.click(screen.getByRole("button", { name: "Reset Filters" })); + await user.click(screen.getByTestId("datatable-clear-filters")); await waitFor(() => - expect(mockUseKeys).toHaveBeenLastCalledWith( - 1, - 50, - expect.objectContaining({ sortBy: "created_at", sortOrder: "desc" }), - ), + expect(mockUseKeys).toHaveBeenLastCalledWith(1, 50, expect.objectContaining({ userID: undefined })), + ); + }); + + it("maps the search box to a server-side key-alias query", async () => { + const user = userEvent.setup(); + mockUseKeys.mockReturnValue({ + data: { keys: [createMockKey()], total_count: 1, current_page: 1, total_pages: 1 }, + isPending: false, + isFetching: false, + refetch: vi.fn(), + } as unknown as ReturnType); + + renderWithProviders(); + + await user.type(await screen.findByTestId("datatable-search"), "check-002"); + + await waitFor( + () => + expect(mockUseKeys).toHaveBeenLastCalledWith(1, 50, expect.objectContaining({ selectedKeyAlias: "check-002" })), + { timeout: 2000 }, ); }); @@ -304,7 +308,7 @@ describe("TeamVirtualKeysTable", () => { }); }); - it("should show No keys found when keys array is empty", async () => { + it("should show the empty state when keys array is empty", async () => { mockUseKeys.mockReturnValue({ data: { keys: [], total_count: 0, current_page: 1, total_pages: 1 } as KeysResponse, isPending: false, @@ -315,26 +319,7 @@ describe("TeamVirtualKeysTable", () => { renderWithProviders(); await waitFor(() => { - expect(screen.getByText("No keys found")).toBeInTheDocument(); - }); - }); - - it("should fetch team-scoped filter options for Key Alias, Organization ID, and User ID", async () => { - const mockFetchTeamFilterOptions = vi.mocked(fetchTeamFilterOptions); - mockFetchTeamFilterOptions.mockResolvedValue({ - keyAliases: ["alice_key_team1", "charlie_key_team1"], - organizationIds: ["org-123"], - userIds: [ - { id: "user-1", email: "alice@example.com" }, - { id: "user-2", email: "charlie@example.com" }, - ], - }); - - // Use unique teamId to avoid cache hit from previous tests (refetchOnMount: false) - renderWithProviders(); - - await waitFor(() => { - expect(mockFetchTeamFilterOptions).toHaveBeenCalledWith("test-token", "team-filter-options-test"); + expect(screen.getByText("No rows match your search or filters.")).toBeInTheDocument(); }); }); diff --git a/ui/litellm-dashboard/src/components/team/TeamVirtualKeysTable.tsx b/ui/litellm-dashboard/src/components/team/TeamVirtualKeysTable.tsx index 73f524e13f8..e008de12dd4 100644 --- a/ui/litellm-dashboard/src/components/team/TeamVirtualKeysTable.tsx +++ b/ui/litellm-dashboard/src/components/team/TeamVirtualKeysTable.tsx @@ -1,21 +1,25 @@ "use client"; import { useKeys } from "@/app/(dashboard)/hooks/keys/useKeys"; import { DateCell, IdCell, MoneyCell } from "@/components/shared/table_cells"; -import { DataTable, DataTablePagination, DataTableSortHeader } from "@/components/shared/DataTable"; +import { + DataTable, + DataTableFilterDrawer, + DataTableFilterField, + DataTableSortHeader, + DataTableToolbar, +} from "@/components/shared/DataTable"; +import { Input } from "@/components/ui/input"; import { ChevronDownIcon, ChevronRightIcon } from "@heroicons/react/outline"; -import { ColumnDef, PaginationState, SortingState } from "@tanstack/react-table"; +import { ColumnDef, ColumnFiltersState, OnChangeFn, PaginationState, SortingState } from "@tanstack/react-table"; import { Badge, Icon, Text } from "@tremor/react"; import { Popover, Tooltip, Typography } from "antd"; +import debounce from "lodash/debounce"; import DefaultProxyAdminTag from "../common_components/DefaultProxyAdminTag"; import React, { useCallback, useEffect, useMemo, useState } from "react"; import { getModelDisplayName } from "../key_team_helpers/fetch_available_models_team_key"; import { KeyResponse, Team } from "../key_team_helpers/key_list"; -import FilterComponent, { FilterOption } from "../molecules/filter"; import { Organization } from "../networking"; import KeyInfoView from "../templates/key_info_view"; -import { useQuery } from "@tanstack/react-query"; -import { fetchTeamFilterOptions } from "../key_team_helpers/filter_helpers"; -import useAuthorized from "@/app/(dashboard)/hooks/useAuthorized"; interface TeamVirtualKeysTableProps { teamId: string; @@ -30,18 +34,41 @@ interface TeamVirtualKeysTableProps { const DEFAULT_SORTING: SortingState = [{ id: "created_at", desc: true }]; export function TeamVirtualKeysTable({ teamId, teamAlias, organization }: TeamVirtualKeysTableProps) { - const { accessToken } = useAuthorized(); const [selectedKey, setSelectedKey] = useState(null); const [sorting, setSorting] = useState(DEFAULT_SORTING); const [tablePagination, setTablePagination] = useState({ pageIndex: 0, pageSize: 50, }); - const [filters, setFilters] = useState>({ - "Organization ID": "", - "Key Alias": "", - "User ID": "", - }); + const [columnFilters, setColumnFilters] = useState([]); + const [filtersOpen, setFiltersOpen] = useState(false); + const [searchInput, setSearchInput] = useState(""); + const [searchQuery, setSearchQuery] = useState(""); + + const debouncedSetSearch = useMemo( + () => + debounce((value: string) => { + setSearchQuery(value); + setTablePagination((prev) => ({ ...prev, pageIndex: 0 })); + }, 300), + [], + ); + useEffect(() => () => debouncedSetSearch.cancel(), [debouncedSetSearch]); + const handleSearchChange = useCallback( + (value: string) => { + setSearchInput(value); + debouncedSetSearch(value); + }, + [debouncedSetSearch], + ); + + const getFilterValue = useCallback( + (columnId: string): string | undefined => { + const entry = columnFilters.find((filter) => filter.id === columnId); + return typeof entry?.value === "string" && entry.value.trim() ? entry.value.trim() : undefined; + }, + [columnFilters], + ); const sortBy = sorting.length > 0 ? sorting[0].id : "created_at"; const sortOrder = sorting.length > 0 ? (sorting[0].desc ? "desc" : "asc") : "desc"; @@ -56,9 +83,8 @@ export function TeamVirtualKeysTable({ teamId, teamAlias, organization }: TeamVi refetch, } = useKeys(pageIndex + 1, pageSize, { teamID: teamId, - organizationID: filters["Organization ID"]?.trim() || undefined, - selectedKeyAlias: filters["Key Alias"]?.trim() || undefined, - userID: filters["User ID"]?.trim() || undefined, + selectedKeyAlias: searchQuery.trim() || undefined, + userID: getFilterValue("user_id"), sortBy: sortBy || undefined, sortOrder: sortOrder || undefined, expand: "user", @@ -95,18 +121,6 @@ export function TeamVirtualKeysTable({ teamId, teamAlias, organization }: TeamVi [teamId, teamAlias, organization], ); - const teamFilterOptionsQuery = useQuery({ - queryKey: ["teamFilterOptions", teamId, accessToken], - queryFn: async () => fetchTeamFilterOptions(accessToken, teamId), - enabled: !!accessToken && !!teamId, - staleTime: 30000, // 30 seconds - align with useKeys - }); - const teamFilterOptions = teamFilterOptionsQuery.data || { - keyAliases: [], - organizationIds: [], - userIds: [], - }; - const handleStorageChange = useCallback(() => { refetch?.(); }, [refetch]); @@ -116,76 +130,17 @@ export function TeamVirtualKeysTable({ teamId, teamAlias, organization }: TeamVi return () => window.removeEventListener("storage", handleStorageChange); }, [handleStorageChange]); - const handleFilterChange = useCallback((newFilters: Record) => { - setFilters((prev) => ({ - ...prev, - "Organization ID": newFilters["Organization ID"] ?? prev["Organization ID"], - "Key Alias": newFilters["Key Alias"] ?? prev["Key Alias"], - "User ID": newFilters["User ID"] ?? prev["User ID"], - })); + const handleColumnFiltersChange = useCallback>((updaterOrValue) => { + setColumnFilters(updaterOrValue); setTablePagination((prev) => ({ ...prev, pageIndex: 0 })); }, []); - const handleFilterReset = useCallback(() => { - setFilters({ - "Organization ID": "", - "Key Alias": "", - "User ID": "", - }); - setSorting(DEFAULT_SORTING); - setTablePagination((prev) => ({ ...prev, pageIndex: 0 })); - }, []); - - const filterOptions: FilterOption[] = useMemo( - () => [ - { - name: "Organization ID", - label: "Organization ID", - isSearchable: true, - searchFn: async (searchText: string) => { - const { organizationIds } = teamFilterOptions; - if (!organizationIds.length) return []; - const lower = searchText.toLowerCase(); - const filtered = lower ? organizationIds.filter((id) => id.toLowerCase().includes(lower)) : organizationIds; - return filtered.map((id) => ({ label: id, value: id })); - }, - }, - { - name: "Key Alias", - label: "Key Alias", - isSearchable: true, - searchFn: async (searchText: string) => { - const { keyAliases } = teamFilterOptions; - const lower = searchText.toLowerCase(); - const filtered = lower ? keyAliases.filter((alias) => alias.toLowerCase().includes(lower)) : keyAliases; - return filtered.map((alias) => ({ label: alias, value: alias })); - }, - }, - { - name: "User ID", - label: "User ID", - isSearchable: true, - searchFn: async (searchText: string) => { - const { userIds } = teamFilterOptions; - const lower = searchText.toLowerCase(); - const filtered = lower - ? userIds.filter((u) => u.id.toLowerCase().includes(lower) || u.email.toLowerCase().includes(lower)) - : userIds; - return filtered.map((u) => ({ - label: u.email ? `${u.id} (${u.email})` : u.id, - value: u.id, - })); - }, - }, - ], - [teamFilterOptions], - ); - const columns: ColumnDef[] = useMemo( () => [ { id: "token", accessorKey: "token", + meta: { title: "Key ID" }, header: ({ column }) => , size: 120, enableSorting: true, @@ -196,6 +151,7 @@ export function TeamVirtualKeysTable({ teamId, teamAlias, organization }: TeamVi { id: "key_alias", accessorKey: "key_alias", + meta: { title: "Key Alias" }, header: ({ column }) => , size: 150, enableSorting: true, @@ -268,6 +224,7 @@ export function TeamVirtualKeysTable({ teamId, teamAlias, organization }: TeamVi { id: "created_at", accessorKey: "created_at", + meta: { title: "Created At" }, header: ({ column }) => , size: 120, enableSorting: true, @@ -335,6 +292,7 @@ export function TeamVirtualKeysTable({ teamId, teamAlias, organization }: TeamVi { id: "updated_at", accessorKey: "updated_at", + meta: { title: "Updated At" }, header: ({ column }) => , size: 120, enableSorting: true, @@ -359,6 +317,7 @@ export function TeamVirtualKeysTable({ teamId, teamAlias, organization }: TeamVi { id: "spend", accessorKey: "spend", + meta: { title: "Spend (USD)" }, header: ({ column }) => , size: 100, enableSorting: true, @@ -367,6 +326,7 @@ export function TeamVirtualKeysTable({ teamId, teamAlias, organization }: TeamVi { id: "max_budget", accessorKey: "max_budget", + meta: { title: "Budget (USD)" }, header: ({ column }) => , size: 110, enableSorting: true, @@ -503,27 +463,7 @@ export function TeamVirtualKeysTable({ teamId, teamAlias, organization }: TeamVi onDelete={refetch} /> ) : ( -
-
- -
- -
- setTablePagination((prev) => ({ ...prev, pageIndex: nextPage }))} - onPageSizeChange={(nextSize) => setTablePagination({ pageIndex: 0, pageSize: nextSize })} - isLoading={isLoading || isFetching} - /> -
- +
null} + filterMode="server" + columnFilters={columnFilters} + onColumnFiltersChange={handleColumnFiltersChange} enableColumnResizing columnResizeMode="onChange" isLoading={isLoading || isFetching} loadingMessage="Loading keys..." - noDataMessage="No keys found" maxBodyHeight="75vh" size="compact" + toolbar={(table) => ( + <> + refetch?.()} + isRefreshing={isFetching} + onOpenFilters={() => setFiltersOpen(true)} + filterLabels={{ user_id: "User ID" }} + /> + + {({ get, set }) => ( + + set("user_id", event.target.value)} + placeholder="Filter by user ID…" + /> + + )} + + + )} />
)} diff --git a/ui/litellm-dashboard/src/components/ui/sheet.tsx b/ui/litellm-dashboard/src/components/ui/sheet.tsx new file mode 100644 index 00000000000..b619c927bbb --- /dev/null +++ b/ui/litellm-dashboard/src/components/ui/sheet.tsx @@ -0,0 +1,100 @@ +"use client"; + +import * as React from "react"; +import { Dialog as SheetPrimitive } from "@base-ui/react/dialog"; + +import { cn } from "@/lib/cva.config"; +import { Button } from "@/components/ui/button"; +import { XIcon } from "lucide-react"; + +function Sheet({ ...props }: SheetPrimitive.Root.Props) { + return ; +} + +function SheetTrigger({ ...props }: SheetPrimitive.Trigger.Props) { + return ; +} + +function SheetClose({ ...props }: SheetPrimitive.Close.Props) { + return ; +} + +function SheetPortal({ ...props }: SheetPrimitive.Portal.Props) { + return ; +} + +function SheetOverlay({ className, ...props }: SheetPrimitive.Backdrop.Props) { + return ( + + ); +} + +function SheetContent({ + className, + children, + side = "right", + showCloseButton = true, + ...props +}: SheetPrimitive.Popup.Props & { + side?: "top" | "right" | "bottom" | "left"; + showCloseButton?: boolean; +}) { + return ( + + + + {children} + {showCloseButton && ( + } + > + + Close + + )} + + + ); +} + +function SheetHeader({ className, ...props }: React.ComponentProps<"div">) { + return
; +} + +function SheetFooter({ className, ...props }: React.ComponentProps<"div">) { + return
; +} + +function SheetTitle({ className, ...props }: SheetPrimitive.Title.Props) { + return ( + + ); +} + +function SheetDescription({ className, ...props }: SheetPrimitive.Description.Props) { + return ( + + ); +} + +export { Sheet, SheetTrigger, SheetClose, SheetContent, SheetHeader, SheetFooter, SheetTitle, SheetDescription }; From ae55170343daf29c700b8dd504be303d66915376 Mon Sep 17 00:00:00 2001 From: Yuneng Jiang Date: Fri, 10 Jul 2026 18:05:49 -0700 Subject: [PATCH 14/99] fix(ui): show filter-select label and shape loading skeletons per column The workflow-runs Status filter leaked the internal "__all__" sentinel as its displayed value because Base UI's Select.Value renders the raw value when no items map or children function is given. Drop the sentinel and use Base UI's native null handling: a null "All statuses" item plus a placeholder, with an items map so a real selection renders its capitalized label rather than the raw status string The shared DataTable rendered every loading-skeleton cell as one identical half-width bar, which read as a rigid grid instead of the table beneath it. Vary the skeleton width per column and add a per-column skeleton shape hint (text or twoLine) on ColumnMeta so identity columns like the workflow "Run" cell get a two-line skeleton that matches their real content --- .../(dashboard)/workflows/WorkflowRuns.tsx | 23 ++++++++----- .../shared/DataTable/DataTable.test.tsx | 17 ++++++++++ .../components/shared/DataTable/DataTable.tsx | 32 ++++++++++++++----- .../components/shared/DataTable/columnMeta.ts | 3 +- .../src/components/shared/DataTable/types.ts | 1 + 5 files changed, 59 insertions(+), 17 deletions(-) diff --git a/ui/litellm-dashboard/src/app/(dashboard)/workflows/WorkflowRuns.tsx b/ui/litellm-dashboard/src/app/(dashboard)/workflows/WorkflowRuns.tsx index 119e61c0222..9afa07251c2 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/workflows/WorkflowRuns.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/workflows/WorkflowRuns.tsx @@ -67,7 +67,13 @@ const STATUS_DOT: Record = { }; const RUN_STATUS_OPTIONS: RunStatus[] = ["pending", "running", "paused", "completed", "failed"]; -const FILTER_ALL = "__all__"; +const STATUS_LABELS: Record = { + pending: "Pending", + running: "Running", + paused: "Paused", + completed: "Completed", + failed: "Failed", +}; const EVENT_COLOR: Record = { "step.started": { bar: "#f0fdf4", border: "#86efac", text: "#16a34a" }, @@ -562,7 +568,7 @@ const WorkflowRuns: React.FC = ({ accessToken }) => { id: "run", accessorFn: (row) => `${runTitle(row)} ${row.run_id}`, header: "Run", - meta: { title: "Run" }, + meta: { title: "Run", skeleton: "twoLine" }, cell: ({ row }) => { const run = row.original; return ( @@ -674,17 +680,18 @@ const WorkflowRuns: React.FC = ({ accessToken }) => { <> - {guardrailSettings?.supported_modes?.map((mode) => ( + {getSupportedModesForProvider(guardrailSettings, selectedProvider)?.map((mode) => (