mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-26 01:12:21 +00:00
fix(ui): gate MCP catalog on prompts or resources 401 and rename tab to MCP Catalog
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This commit is contained in:
parent
6030fcc01d
commit
6b5c4634d0
6 changed files with 86 additions and 40 deletions
|
|
@ -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<void> {
|
||||
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();
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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 });
|
||||
|
|
|
|||
|
|
@ -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();
|
||||
});
|
||||
|
|
|
|||
|
|
@ -141,7 +141,7 @@ export const MCPServerView: React.FC<MCPServerViewProps> = ({
|
|||
Overview
|
||||
</TabsTrigger>
|
||||
<TabsTrigger value="1" className="flex-none rounded-none px-4 py-2">
|
||||
MCP Tools
|
||||
MCP Catalog
|
||||
</TabsTrigger>
|
||||
{isProxyAdmin && (
|
||||
<TabsTrigger value="2" className="flex-none rounded-none px-4 py-2">
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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 = ({
|
|||
<div className="grid h-auto w-full grid-cols-4 gap-4">
|
||||
{/* Left Sidebar with Controls */}
|
||||
<div className="col-span-1 flex flex-col bg-muted p-4">
|
||||
<h2 className="mt-2 mb-6 text-xl font-semibold">MCP Tools</h2>
|
||||
<h2 className="mt-2 mb-6 text-xl font-semibold">MCP Catalog</h2>
|
||||
|
||||
<div className="flex flex-col flex-1">
|
||||
{/* 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
|
||||
</Button>
|
||||
</div>
|
||||
)}
|
||||
|
|
@ -422,7 +424,9 @@ const MCPToolsViewer = ({
|
|||
<div className="rounded-lg border border-border bg-card p-4 text-center">
|
||||
<Lock className="mx-auto mb-2 size-6 text-muted-foreground" />
|
||||
<p className="mb-1 text-xs font-medium">Authentication required</p>
|
||||
<p className="mb-3 text-xs text-muted-foreground">Authenticate to view available tools</p>
|
||||
<p className="mb-3 text-xs text-muted-foreground">
|
||||
Authenticate to view available tools, prompts, and resources
|
||||
</p>
|
||||
<Button
|
||||
size="sm"
|
||||
onClick={startOAuthFlow}
|
||||
|
|
@ -444,7 +448,7 @@ const MCPToolsViewer = ({
|
|||
<Lock className="mx-auto mb-2 size-6 text-muted-foreground" />
|
||||
<p className="mb-1 text-xs font-medium">Authentication required</p>
|
||||
<p className="mb-3 text-xs text-muted-foreground">
|
||||
Authenticate with the upstream provider to view available tools
|
||||
Authenticate with the upstream provider to view available tools, prompts, and resources
|
||||
</p>
|
||||
<Button
|
||||
size="sm"
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue