diff --git a/tests/e2e/ui/helpers/mcp.ts b/tests/e2e/ui/helpers/mcp.ts index 554177e11bc..178644df95f 100644 --- a/tests/e2e/ui/helpers/mcp.ts +++ b/tests/e2e/ui/helpers/mcp.ts @@ -55,9 +55,9 @@ export async function deleteMcpServerByName(page: PwPage, serverName: string): P } } -/** Opens a server from the grid and switches to its MCP Tools tab. */ +/** Opens a server from the grid and switches to its MCP Catalog tab. */ export async function openMcpToolsTab(page: PwPage, serverName: string): Promise { await page.getByTestId("mcp-servers-grid").getByText(serverName).first().click(); await expect(page.getByRole("button", { name: /Back to All Servers/i })).toBeVisible({ timeout: 10_000 }); - await page.getByRole("tab", { name: "MCP Tools" }).click(); + await page.getByRole("tab", { name: "MCP Catalog" }).click(); } diff --git a/tests/e2e/ui/tests/mcp/mcpTools.spec.ts b/tests/e2e/ui/tests/mcp/mcpTools.spec.ts index 225ca8b9449..3e6093c5c3f 100644 --- a/tests/e2e/ui/tests/mcp/mcpTools.spec.ts +++ b/tests/e2e/ui/tests/mcp/mcpTools.spec.ts @@ -35,7 +35,7 @@ test.describe("MCP Tools", () => { await deleteMcpServerByName(page, serverName); }); - test("MCP Tools tab lists the tools the upstream server advertises", async ({ page }) => { + test("MCP Catalog tab lists the tools the upstream server advertises", async ({ page }) => { // Fetched through the proxy on mount, so allow for a cold upstream connection. const toolList = page.locator(".mcp-tools-scrollable"); await expect(toolList).toBeVisible({ timeout: 30_000 }); diff --git a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/mcp_server_view.test.tsx b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/mcp_server_view.test.tsx index da564f23de5..f45bf8ea5da 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/mcp_server_view.test.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/mcp_server_view.test.tsx @@ -111,7 +111,7 @@ describe("MCPServerView", () => { it("opens the tools viewer on the MCP Tools tab", async () => { renderView(); - await userEvent.click(screen.getByRole("tab", { name: "MCP Tools" })); + await userEvent.click(screen.getByRole("tab", { name: "MCP Catalog" })); expect(await screen.findByText("tools viewer")).toBeInTheDocument(); }); diff --git a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/mcp_server_view.tsx b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/mcp_server_view.tsx index a346c7d986b..09b037e2c99 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/mcp_server_view.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/mcp_server_view.tsx @@ -141,7 +141,7 @@ export const MCPServerView: React.FC = ({ Overview - MCP Tools + MCP Catalog {isProxyAdmin && ( diff --git a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/mcp_tools.test.tsx b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/mcp_tools.test.tsx index f76613d9a5b..7d785e64d05 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/mcp_tools.test.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/mcp_tools.test.tsx @@ -1,4 +1,5 @@ -import { act, render, screen, waitFor, within } from "@testing-library/react"; +import { render, screen, waitFor, within } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; import { describe, expect, it, vi, beforeEach } from "vitest"; import MCPToolsViewer from "./mcp_tools"; @@ -8,7 +9,7 @@ import { listMCPResources, getMCPOAuthUserCredentialStatus, } from "@/components/networking"; -import { isTokenValid, getToken } from "@/utils/mcpTokenStore"; +import { isTokenValid, getToken, removeToken } from "@/utils/mcpTokenStore"; vi.mock("@/components/networking", () => ({ listMCPTools: vi.fn(), @@ -24,14 +25,14 @@ vi.mock("@/utils/mcpTokenStore", () => ({ removeToken: vi.fn(), })); -const { toolsOAuthFlowSpy, userMcpOAuthFlowSpy } = vi.hoisted(() => ({ - toolsOAuthFlowSpy: vi.fn(() => ({ startOAuthFlow: vi.fn(), status: "idle", error: null })), - userMcpOAuthFlowSpy: vi.fn((_options: { onSuccess: () => void }) => ({ - startOAuthFlow: vi.fn(), - status: "idle", - error: null, - })), -})); +const { toolsOAuthFlowSpy, userMcpOAuthFlowSpy } = vi.hoisted(() => { + type FlowState = { startOAuthFlow: () => void; status: string; error: string | null }; + const idle = (): FlowState => ({ startOAuthFlow: () => {}, status: "idle", error: null }); + return { + toolsOAuthFlowSpy: vi.fn((_options: { onSuccess: (token: string) => void }) => idle()), + userMcpOAuthFlowSpy: vi.fn((_options: { onSuccess: () => void }) => idle()), + }; +}); vi.mock("@/hooks/useToolsOAuthFlow", () => ({ useToolsOAuthFlow: toolsOAuthFlowSpy, @@ -321,27 +322,68 @@ describe("MCPToolsViewer prompts and resources catalog", () => { expect(await screen.findByText("summarize")).toBeInTheDocument(); }); - it("reloads prompts and resources together with tools after the user re-authorizes", async () => { - const expiredPrompts = { - prompts: [], - error: "auth_required", - message: "upstream credential expired", - status: 401, - }; - vi.mocked(listMCPPrompts).mockResolvedValue(expiredPrompts); - userMcpOAuthFlowSpy.mockClear(); + const unauthorized = { error: "auth_required", message: "upstream credential expired", status: 401 }; + + it("gates on a prompts 401 even when tools load, and reloads all three listings after Authorize (stored credential)", async () => { + vi.mocked(listMCPTools).mockResolvedValue({ tools: [], error: null }); + vi.mocked(listMCPPrompts).mockResolvedValue({ prompts: [], ...unauthorized }); + // The hook completes the redirect flow out of band; here Authorize resolves it immediately. + userMcpOAuthFlowSpy.mockReset().mockImplementation((options) => ({ + startOAuthFlow: () => options.onSuccess(), + status: "idle", + error: null, + })); renderViewer({ oauth2_flow: null, delegate_auth_to_upstream: false }); - const prompts = await screen.findByRole("region", { name: "Prompts" }); - expect(await within(prompts).findByText("Error: upstream credential expired")).toBeInTheDocument(); + expect(await screen.findByText(GATE_TEXT)).toBeInTheDocument(); + expect(screen.queryByRole("region", { name: "Prompts" })).not.toBeInTheDocument(); + expect(vi.mocked(listMCPTools)).toHaveBeenCalledTimes(1); expect(vi.mocked(listMCPPrompts)).toHaveBeenCalledTimes(1); + expect(vi.mocked(listMCPResources)).toHaveBeenCalledTimes(1); vi.mocked(listMCPPrompts).mockResolvedValue({ prompts: [{ name: "summarize" }] }); - act(() => userMcpOAuthFlowSpy.mock.calls.at(-1)?.[0].onSuccess()); + await userEvent.click(screen.getByRole("button", { name: "Authorize" })); + const prompts = await screen.findByRole("region", { name: "Prompts" }); expect(await within(prompts).findByText("summarize")).toBeInTheDocument(); + expect(screen.queryByText(GATE_TEXT)).not.toBeInTheDocument(); expect(vi.mocked(listMCPTools)).toHaveBeenCalledTimes(2); + expect(vi.mocked(listMCPPrompts)).toHaveBeenCalledTimes(2); expect(vi.mocked(listMCPResources)).toHaveBeenCalledTimes(2); }); + + it("gates on a resources 401 even when tools load, and relists all three with the new browser token after Authorize", async () => { + vi.mocked(isTokenValid).mockReturnValue(true); + vi.mocked(getToken).mockReturnValue({ + access_token: "stale-tok", + expires_at: Date.now() + 60_000, + token_type: "bearer", + }); + vi.mocked(listMCPResources).mockResolvedValue({ resources: [], resource_templates: [], ...unauthorized }); + toolsOAuthFlowSpy.mockReset().mockImplementation((options) => ({ + startOAuthFlow: () => options.onSuccess("fresh-tok"), + status: "idle", + error: null, + })); + + renderViewer({ oauth2_flow: null, delegate_auth_to_upstream: true }); + + expect(await screen.findByText(GATE_TEXT)).toBeInTheDocument(); + expect(vi.mocked(removeToken)).toHaveBeenCalledWith("srv-1", "tin@berri.ai"); + expect(screen.queryByRole("region", { name: "Resources" })).not.toBeInTheDocument(); + + vi.mocked(listMCPResources).mockResolvedValue({ + resources: [{ name: "readme", uri: "demo://readme" }], + resource_templates: [], + }); + await userEvent.click(screen.getByRole("button", { name: "Authorize" })); + + const resources = await screen.findByRole("region", { name: "Resources" }); + expect(await within(resources).findByText("demo://readme")).toBeInTheDocument(); + const freshHeader = expect.objectContaining({ "x-mcp-slack-authorization": "Bearer fresh-tok" }); + expect(vi.mocked(listMCPTools)).toHaveBeenLastCalledWith("litellm-key", "srv-1", freshHeader); + expect(vi.mocked(listMCPPrompts)).toHaveBeenLastCalledWith("litellm-key", "srv-1", freshHeader); + expect(vi.mocked(listMCPResources)).toHaveBeenLastCalledWith("litellm-key", "srv-1", freshHeader); + }); }); diff --git a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/mcp_tools.tsx b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/mcp_tools.tsx index 4be58462a7a..5178ba26079 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/mcp_tools.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/mcp_tools.tsx @@ -229,6 +229,12 @@ const MCPToolsViewer = ({ refetchResources(); }, [refetchTools, refetchPrompts, refetchResources]); + const toolsError = mcpToolsError as (Error & { status?: number; response?: { status?: number } }) | null; + const catalogUnauthorized = + (toolsError?.status ?? toolsError?.response?.status) === 401 || + mcpPromptsResponse?.status === 401 || + mcpResourcesResponse?.status === 401; + // authorization_code authorize: same redirect+exchange flow as the admin "Authorize & Fetch" // and the chat "Connect" button, but persists the token to the per-user DB. const onAuthorizationCodeAuthSuccess = useCallback(() => { @@ -256,16 +262,14 @@ const MCPToolsViewer = ({ startDbOAuthFlow(); }, [serverId, startDbOAuthFlow]); - // If the tools query fails with 401, the cached OAuth token is invalid — - // clear it so the auth gate is shown again and the user can re-authenticate. + // A 401 from any listing means the cached OAuth token is invalid; clear it so the + // auth gate is shown again and the user can re-authenticate. useEffect(() => { - const err = mcpToolsError as (Error & { status?: number; response?: { status?: number } }) | null; - const status = err?.status ?? err?.response?.status; - if (status === 401) { + if (catalogUnauthorized) { removeToken(serverId, userID); setOauthToken(null); } - }, [mcpToolsError, serverId, userID]); + }, [catalogUnauthorized, serverId, userID]); // Mutation for calling a tool const { mutate: executeTool, isPending: isCallingTool } = useMutation({ @@ -298,12 +302,10 @@ const MCPToolsViewer = ({ const toolsData = mcpToolsResponse?.tools || []; - const toolsError = mcpToolsError as (Error & { status?: number; response?: { status?: number } }) | null; // authorization_code only: a 401 from the list call means the stored credential is unusable and // the backend's refresh could not mint a token, so the user must re-authorize (the browser flow). // token_exchange has no gateway-side authorize step, so it is not gated here. - const authorizationCodeTokenRejected = - isAuthorizationCode && (toolsError?.status ?? toolsError?.response?.status) === 401; + const authorizationCodeTokenRejected = isAuthorizationCode && catalogUnauthorized; // An auth gate replaces the tool list when the user must authenticate first: // passthrough needs a browser token; authorization_code needs a stored DB credential or a @@ -337,7 +339,7 @@ const MCPToolsViewer = ({
{/* Left Sidebar with Controls */}
-

MCP Tools

+

MCP Catalog

{/* Extra Headers Input Section */} @@ -390,7 +392,7 @@ const MCPToolsViewer = ({ disabled={Object.values(passthroughHeaders).every((v) => !v || !v.trim())} className="mt-2 w-full" > - Load Tools + Load Catalog
)} @@ -422,7 +424,9 @@ const MCPToolsViewer = ({

Authentication required

-

Authenticate to view available tools

+

+ Authenticate to view available tools, prompts, and resources +