mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-14 23:21:35 +00:00
fix(mcp): refresh tools when editing server connection settings
This commit is contained in:
parent
6b721de3e5
commit
892d20d86f
5 changed files with 285 additions and 15 deletions
|
|
@ -0,0 +1,87 @@
|
|||
import { describe, expect, it } from "vitest";
|
||||
import { getEditToolPreview } from "./editToolPreview";
|
||||
|
||||
const saved = {
|
||||
url: "https://example.com/mcp",
|
||||
transport: "http",
|
||||
auth_type: "basic",
|
||||
static_headers: [{ header: "X-Tenant", value: "original" }],
|
||||
};
|
||||
|
||||
describe("getEditToolPreview", () => {
|
||||
it("keeps saved discovery for unchanged settings and unrelated edits", () => {
|
||||
expect(getEditToolPreview({ ...saved, server_name: "renamed" }, saved)).toEqual({ kind: "saved" });
|
||||
});
|
||||
|
||||
it("previews URL changes with the existing server credential left for server-side inheritance", () => {
|
||||
expect(getEditToolPreview({ ...saved, url: "https://correct.example/mcp" }, saved)).toEqual({
|
||||
kind: "preview",
|
||||
config: {
|
||||
url: "https://correct.example/mcp",
|
||||
transport: "http",
|
||||
auth_type: "basic",
|
||||
static_headers: { "X-Tenant": "original" },
|
||||
credentials: undefined,
|
||||
},
|
||||
});
|
||||
});
|
||||
|
||||
it("uses edited static headers and only the static auth value", () => {
|
||||
expect(
|
||||
getEditToolPreview(
|
||||
{
|
||||
...saved,
|
||||
static_headers: [{ header: "X-Tenant", value: "corrected" }],
|
||||
credentials: { auth_value: "user:password", access_token: "old-oauth-token", client_secret: "old-client" },
|
||||
},
|
||||
saved,
|
||||
),
|
||||
).toEqual({
|
||||
kind: "preview",
|
||||
config: {
|
||||
url: saved.url,
|
||||
transport: "http",
|
||||
auth_type: "basic",
|
||||
static_headers: { "X-Tenant": "corrected" },
|
||||
credentials: { auth_value: "user:password" },
|
||||
},
|
||||
});
|
||||
});
|
||||
|
||||
it.each(["", "https://", "file:///tmp/server"])("does not connect to an incomplete or unsupported URL: %s", (url) => {
|
||||
expect(getEditToolPreview({ ...saved, url }, saved)).toEqual({ kind: "incomplete" });
|
||||
});
|
||||
|
||||
it("waits for a static header value before connecting", () => {
|
||||
const values = { ...saved, static_headers: [{ header: "X-Tenant", value: "" }] };
|
||||
expect(getEditToolPreview(values, saved)).toEqual({ kind: "incomplete" });
|
||||
});
|
||||
|
||||
it("waits for credentials when switching from None to Basic Auth", () => {
|
||||
expect(getEditToolPreview(saved, { ...saved, auth_type: "none" })).toEqual({ kind: "incomplete" });
|
||||
});
|
||||
|
||||
it("does not forward old credentials when switching to None", () => {
|
||||
const result = getEditToolPreview(
|
||||
{ ...saved, auth_type: "none", credentials: { auth_value: "old-secret" } },
|
||||
saved,
|
||||
);
|
||||
expect(result.kind).toBe("preview");
|
||||
if (result.kind === "preview") expect(result.config.credentials).toBeUndefined();
|
||||
});
|
||||
|
||||
it.each(["oauth2", "true_passthrough", "oauth_delegate", "oauth2_token_exchange", "oauth2_id_jag", "aws_sigv4"])(
|
||||
"preserves the existing discovery path for %s",
|
||||
(auth_type) => {
|
||||
expect(getEditToolPreview({ ...saved, auth_type, url: "https://changed.example/mcp" }, saved)).toEqual({
|
||||
kind: "saved",
|
||||
});
|
||||
},
|
||||
);
|
||||
|
||||
it("keeps stdio and OpenAPI on their existing discovery path", () => {
|
||||
for (const transport of ["stdio", "openapi"]) {
|
||||
expect(getEditToolPreview({ ...saved, transport }, saved)).toEqual({ kind: "saved" });
|
||||
}
|
||||
});
|
||||
});
|
||||
|
|
@ -0,0 +1,53 @@
|
|||
import { AUTH_TYPE, TRANSPORT } from "@/components/mcp_tools/types";
|
||||
import { AUTH_TYPES_REQUIRING_AUTH_VALUE, reduceStaticHeaders } from "./createServerPayload";
|
||||
|
||||
const connectionConfig = (values: Readonly<Record<string, unknown>>) => {
|
||||
const credentials = values.credentials;
|
||||
const authValue =
|
||||
credentials && typeof credentials === "object" && "auth_value" in credentials ? credentials.auth_value : undefined;
|
||||
const needsAuthValue =
|
||||
typeof values.auth_type === "string" && AUTH_TYPES_REQUIRING_AUTH_VALUE.includes(values.auth_type);
|
||||
return {
|
||||
url: typeof values.url === "string" ? values.url : "",
|
||||
transport: typeof values.transport === "string" ? values.transport : "",
|
||||
auth_type: typeof values.auth_type === "string" ? values.auth_type : "",
|
||||
static_headers: Object.fromEntries(
|
||||
Object.entries(reduceStaticHeaders(values.static_headers)).sort(([a], [b]) => a.localeCompare(b)),
|
||||
),
|
||||
credentials:
|
||||
needsAuthValue && typeof authValue === "string" && authValue.trim() ? { auth_value: authValue } : undefined,
|
||||
};
|
||||
};
|
||||
|
||||
type EditToolPreview =
|
||||
| { readonly kind: "saved" }
|
||||
| { readonly kind: "incomplete" }
|
||||
| { readonly kind: "preview"; readonly config: ReturnType<typeof connectionConfig> };
|
||||
|
||||
export const getEditToolPreview = (
|
||||
values: Readonly<Record<string, unknown>>,
|
||||
initialValues: Readonly<Record<string, unknown>>,
|
||||
): EditToolPreview => {
|
||||
const staticAuth =
|
||||
values.auth_type === AUTH_TYPE.NONE ||
|
||||
(typeof values.auth_type === "string" && AUTH_TYPES_REQUIRING_AUTH_VALUE.includes(values.auth_type));
|
||||
if (!staticAuth || ![TRANSPORT.HTTP, TRANSPORT.SSE].includes(String(values.transport))) {
|
||||
return { kind: "saved" };
|
||||
}
|
||||
|
||||
const config = connectionConfig(values);
|
||||
if (JSON.stringify(config) === JSON.stringify(connectionConfig(initialValues))) {
|
||||
return { kind: "saved" };
|
||||
}
|
||||
|
||||
const missingNewCredential =
|
||||
config.auth_type !== initialValues.auth_type &&
|
||||
AUTH_TYPES_REQUIRING_AUTH_VALUE.includes(config.auth_type) &&
|
||||
config.credentials === undefined;
|
||||
const validUrl = URL.canParse(config.url) && ["http:", "https:"].includes(new URL(config.url).protocol);
|
||||
const incompleteHeaders = Object.values(config.static_headers).some((value) => !value.trim());
|
||||
if (!validUrl || missingNewCredential || incompleteHeaders) {
|
||||
return { kind: "incomplete" };
|
||||
}
|
||||
return { kind: "preview", config };
|
||||
};
|
||||
|
|
@ -1,6 +1,9 @@
|
|||
import React from "react";
|
||||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
import { render, screen, waitFor, act } from "@testing-library/react";
|
||||
import { render, screen, waitFor, act, fireEvent } from "@testing-library/react";
|
||||
import userEvent from "@testing-library/user-event";
|
||||
|
||||
import { selectOption } from "./testUtils";
|
||||
|
||||
import MCPServerEdit from "./mcp_server_edit";
|
||||
import * as networking from "@/components/networking";
|
||||
|
|
@ -27,10 +30,6 @@ vi.mock("./mcp_server_cost_config", () => ({
|
|||
default: () => <div data-testid="mcp-cost-config" />,
|
||||
}));
|
||||
|
||||
vi.mock("./mcp_tool_configuration", () => ({
|
||||
default: () => <div data-testid="mcp-tool-config" />,
|
||||
}));
|
||||
|
||||
const BASE: MCPServer = {
|
||||
server_id: "srv_1",
|
||||
server_name: "srv",
|
||||
|
|
@ -369,3 +368,105 @@ describe("mcp_server_edit save payload contract", () => {
|
|||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe("MCPServerEdit live tool preview", () => {
|
||||
beforeEach(() => {
|
||||
vi.resetAllMocks();
|
||||
vi.mocked(networking.listMCPTools).mockResolvedValue({
|
||||
tools: [],
|
||||
error: "connection_error",
|
||||
message: "Saved credentials rejected",
|
||||
});
|
||||
vi.mocked(networking.testMCPToolsListRequest).mockResolvedValue({
|
||||
tools: [
|
||||
{ name: "echo", description: "Echo the supplied message", inputSchema: { type: "object", properties: {} } },
|
||||
],
|
||||
});
|
||||
});
|
||||
|
||||
const renderEditor = (server: MCPServer = BASE) =>
|
||||
render(
|
||||
<MCPServerEdit
|
||||
mcpServer={server}
|
||||
accessToken="access-token"
|
||||
userID="user-1"
|
||||
onCancel={vi.fn()}
|
||||
onSuccess={vi.fn()}
|
||||
availableAccessGroups={[]}
|
||||
/>,
|
||||
);
|
||||
|
||||
it("replaces the saved connection failure with tools after correcting Basic Auth without saving", async () => {
|
||||
renderEditor();
|
||||
expect(await screen.findByText("Saved credentials rejected")).toBeInTheDocument();
|
||||
await selectOption("Authentication", "Basic Auth");
|
||||
fireEvent.change(screen.getByLabelText("Authentication Value"), { target: { value: "preview:correct" } });
|
||||
expect(screen.queryByText("Saved credentials rejected")).not.toBeInTheDocument();
|
||||
expect(screen.getByText("Loading tools...")).toBeInTheDocument();
|
||||
expect(networking.testMCPToolsListRequest).not.toHaveBeenCalled();
|
||||
fireEvent.click(await screen.findByRole("button", { name: "Flat List" }));
|
||||
expect(screen.getByText("echo")).toBeInTheDocument();
|
||||
const expectedConfig = {
|
||||
server_id: BASE.server_id,
|
||||
url: BASE.url,
|
||||
auth_type: "basic",
|
||||
credentials: { auth_value: "preview:correct" },
|
||||
};
|
||||
expect(networking.testMCPToolsListRequest).toHaveBeenCalledExactlyOnceWith(
|
||||
"access-token",
|
||||
expect.objectContaining(expectedConfig),
|
||||
);
|
||||
expect(networking.updateMCPServer).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("refreshes tools when a static header is corrected", async () => {
|
||||
renderEditor({ ...BASE, static_headers: { "X-Preview-Key": "wrong" } });
|
||||
expect(await screen.findByText("Saved credentials rejected")).toBeInTheDocument();
|
||||
fireEvent.change(screen.getByPlaceholderText("Header value"), { target: { value: "correct" } });
|
||||
fireEvent.click(await screen.findByRole("button", { name: "Flat List" }));
|
||||
expect(screen.getByText("echo")).toBeInTheDocument();
|
||||
expect(networking.testMCPToolsListRequest).toHaveBeenCalledExactlyOnceWith(
|
||||
"access-token",
|
||||
expect.objectContaining({ static_headers: { "X-Preview-Key": "correct" } }),
|
||||
);
|
||||
});
|
||||
|
||||
it("coalesces URL edits and ignores an older failed preview after the latest preview succeeds", async () => {
|
||||
const user = userEvent.setup();
|
||||
const older = Promise.withResolvers<{ tools: never[]; error: string; message: string }>();
|
||||
vi.mocked(networking.testMCPToolsListRequest).mockImplementationOnce(() => older.promise);
|
||||
renderEditor();
|
||||
expect(await screen.findByText("Saved credentials rejected")).toBeInTheDocument();
|
||||
fireEvent.change(screen.getByLabelText("MCP Server URL"), { target: { value: "https://first.example/mcp" } });
|
||||
await waitFor(() => expect(networking.testMCPToolsListRequest).toHaveBeenCalledTimes(1));
|
||||
await user.clear(screen.getByLabelText("MCP Server URL"));
|
||||
await user.type(screen.getByLabelText("MCP Server URL"), "https://latest.example/mcp");
|
||||
expect(networking.testMCPToolsListRequest).toHaveBeenCalledTimes(1);
|
||||
fireEvent.click(await screen.findByRole("button", { name: "Flat List" }));
|
||||
expect(screen.getByText("echo")).toBeInTheDocument();
|
||||
expect(networking.testMCPToolsListRequest).toHaveBeenCalledTimes(2);
|
||||
expect(networking.testMCPToolsListRequest).toHaveBeenLastCalledWith(
|
||||
"access-token",
|
||||
expect.objectContaining({ url: "https://latest.example/mcp" }),
|
||||
);
|
||||
await act(async () => older.resolve({ tools: [], error: "connection_error", message: "Older request failed" }));
|
||||
expect(screen.getByText("echo")).toBeInTheDocument();
|
||||
expect(screen.queryByText("Older request failed")).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("ignores a saved-record response after editing and restores saved discovery when changes are reverted", async () => {
|
||||
const saved = Promise.withResolvers<{ tools: never[]; error: string; message: string }>();
|
||||
vi.mocked(networking.listMCPTools).mockImplementationOnce(() => saved.promise);
|
||||
renderEditor();
|
||||
await waitFor(() => expect(networking.listMCPTools).toHaveBeenCalledTimes(1));
|
||||
fireEvent.change(screen.getByLabelText("MCP Server URL"), { target: { value: "https://correct.example/mcp" } });
|
||||
fireEvent.click(await screen.findByRole("button", { name: "Flat List" }));
|
||||
expect(screen.getByText("echo")).toBeInTheDocument();
|
||||
await act(async () => saved.resolve({ tools: [], error: "connection_error", message: "Stale saved response" }));
|
||||
expect(screen.getByText("echo")).toBeInTheDocument();
|
||||
expect(screen.queryByText("Stale saved response")).not.toBeInTheDocument();
|
||||
fireEvent.change(screen.getByLabelText("MCP Server URL"), { target: { value: BASE.url } });
|
||||
expect(await screen.findByText("Saved credentials rejected")).toBeInTheDocument();
|
||||
expect(networking.listMCPTools).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -52,6 +52,7 @@ import EnvVarsSection from "./EnvVarsSection";
|
|||
import { validateMCPServerUrl, validateMCPServerName, normalizeToolOverrideMap } from "./utils";
|
||||
import { EditServerFormValues, buildEditServerPayload, editPayloadErrorMessage } from "./editServerPayload";
|
||||
import { toast } from "@/lib/toast";
|
||||
import { getEditToolPreview } from "./editToolPreview";
|
||||
import { useMcpOAuthFlow } from "@/hooks/useMcpOAuthFlow";
|
||||
import {
|
||||
MountedFormField,
|
||||
|
|
@ -449,14 +450,27 @@ const MCPServerEdit: React.FC<MCPServerEditProps> = ({
|
|||
}
|
||||
}, [mcpServer]);
|
||||
|
||||
// Fetch tools when component mounts for a saved server
|
||||
const toolPreview = getEditToolPreview(allFieldsValue(form), initialValues);
|
||||
const toolPreviewKey = JSON.stringify(toolPreview);
|
||||
|
||||
useEffect(() => {
|
||||
if (!mcpServer.server_id || mcpServer.server_id.trim() === "") {
|
||||
let active = true;
|
||||
setTools([]);
|
||||
setToolsError(null);
|
||||
setIsLoadingTools(false);
|
||||
if (!accessToken || !mcpServer.server_id) return;
|
||||
if (toolPreview.kind === "incomplete") {
|
||||
setToolsError("Complete the URL, authentication, and header settings to load tools.");
|
||||
return;
|
||||
}
|
||||
fetchTools();
|
||||
setIsLoadingTools(true);
|
||||
const timer = setTimeout(() => fetchTools(() => active), toolPreview.kind === "preview" ? 500 : 0);
|
||||
return () => {
|
||||
active = false;
|
||||
clearTimeout(timer);
|
||||
};
|
||||
// eslint-disable-next-line react-hooks/exhaustive-deps
|
||||
}, [mcpServer, accessToken, userID, oauthTokenResponse?.access_token]);
|
||||
}, [mcpServer, accessToken, userID, oauthTokenResponse?.access_token, toolPreviewKey]);
|
||||
|
||||
// Invalidate a token authorized in this edit session once any mint-relevant field diverges from the
|
||||
// identity it was minted against (url, auth_type, oauth_flow_type, client creds/scopes, or the
|
||||
|
|
@ -519,6 +533,7 @@ const MCPServerEdit: React.FC<MCPServerEditProps> = ({
|
|||
const previewWithStagedInteractiveToken = async (
|
||||
isPassthrough: boolean,
|
||||
isBrowserHeldTokenMode: boolean,
|
||||
isCurrent: () => boolean,
|
||||
): Promise<boolean> => {
|
||||
const stagedToken =
|
||||
!isPassthrough && !isBrowserHeldTokenMode && getEffectiveAuthType() === AUTH_TYPE.OAUTH2
|
||||
|
|
@ -550,6 +565,7 @@ const MCPServerEdit: React.FC<MCPServerEditProps> = ({
|
|||
registration_url: values.registration_url,
|
||||
};
|
||||
const toolsResponse = await testMCPToolsListRequest(accessToken, previewConfig, stagedToken);
|
||||
if (!isCurrent()) return true;
|
||||
if (toolsResponse.tools && !toolsResponse.error) {
|
||||
setTools(toolsResponse.tools);
|
||||
} else {
|
||||
|
|
@ -557,15 +573,16 @@ const MCPServerEdit: React.FC<MCPServerEditProps> = ({
|
|||
setToolsError(toolsResponse.message || "Failed to load tools");
|
||||
}
|
||||
} catch (error) {
|
||||
if (!isCurrent()) return true;
|
||||
setTools([]);
|
||||
setToolsError(error instanceof Error ? error.message : "Failed to load tools");
|
||||
} finally {
|
||||
setIsLoadingTools(false);
|
||||
if (isCurrent()) setIsLoadingTools(false);
|
||||
}
|
||||
return true;
|
||||
};
|
||||
|
||||
const fetchTools = async () => {
|
||||
const fetchTools = async (isCurrent: () => boolean) => {
|
||||
if (!accessToken || !mcpServer.server_id) return;
|
||||
|
||||
// OBO/M2M/static auth is attached server-side from the stored credential, so
|
||||
|
|
@ -574,6 +591,7 @@ const MCPServerEdit: React.FC<MCPServerEditProps> = ({
|
|||
// same way the Tools playground does.
|
||||
let customHeaders: Record<string, string> | undefined;
|
||||
const isPassthrough =
|
||||
toolPreview.kind === "saved" &&
|
||||
getMcpOAuthMode({
|
||||
auth_type: mcpServer.auth_type,
|
||||
oauth2_flow: mcpServer.oauth2_flow,
|
||||
|
|
@ -581,9 +599,10 @@ const MCPServerEdit: React.FC<MCPServerEditProps> = ({
|
|||
}) === "passthrough";
|
||||
const isBrowserHeldTokenMode = isClientForwardedTokenMode(getEffectiveAuthType());
|
||||
|
||||
if (await previewWithStagedInteractiveToken(isPassthrough, isBrowserHeldTokenMode)) {
|
||||
if (await previewWithStagedInteractiveToken(isPassthrough, isBrowserHeldTokenMode, isCurrent)) {
|
||||
return;
|
||||
}
|
||||
if (!isCurrent()) return;
|
||||
if (isPassthrough || isBrowserHeldTokenMode) {
|
||||
const token =
|
||||
oauthTokenResponse?.access_token ??
|
||||
|
|
@ -591,6 +610,7 @@ const MCPServerEdit: React.FC<MCPServerEditProps> = ({
|
|||
? getToken(mcpServer.server_id, userID)?.access_token ?? null
|
||||
: null);
|
||||
if (!token) {
|
||||
setIsLoadingTools(false);
|
||||
setTools([]);
|
||||
setToolsError(
|
||||
isBrowserHeldTokenMode
|
||||
|
|
@ -608,7 +628,15 @@ const MCPServerEdit: React.FC<MCPServerEditProps> = ({
|
|||
try {
|
||||
// include_disabled_tools: configuring the allowlist needs the full server
|
||||
// catalog, so tools toggled off still render (as unchecked) instead of vanishing.
|
||||
const toolsResponse = await listMCPTools(accessToken, mcpServer.server_id, customHeaders, true);
|
||||
const toolsResponse =
|
||||
toolPreview.kind === "preview"
|
||||
? await testMCPToolsListRequest(accessToken, {
|
||||
...toolPreview.config,
|
||||
server_id: mcpServer.server_id,
|
||||
server_name: mcpServer.server_name || mcpServer.alias,
|
||||
})
|
||||
: await listMCPTools(accessToken, mcpServer.server_id, customHeaders, true);
|
||||
if (!isCurrent()) return;
|
||||
|
||||
if (toolsResponse.tools && !toolsResponse.error) {
|
||||
setTools(toolsResponse.tools);
|
||||
|
|
@ -617,10 +645,11 @@ const MCPServerEdit: React.FC<MCPServerEditProps> = ({
|
|||
setToolsError(toolsResponse.message || "Failed to load tools");
|
||||
}
|
||||
} catch (error) {
|
||||
if (!isCurrent()) return;
|
||||
setTools([]);
|
||||
setToolsError(error instanceof Error ? error.message : "Failed to load tools");
|
||||
} finally {
|
||||
setIsLoadingTools(false);
|
||||
if (isCurrent()) setIsLoadingTools(false);
|
||||
}
|
||||
};
|
||||
|
||||
|
|
|
|||
|
|
@ -433,7 +433,7 @@ const MCPToolConfiguration: React.FC<MCPToolConfigurationProps> = ({
|
|||
{isLoadingTools && (
|
||||
<div className="flex items-center justify-center gap-3 py-6">
|
||||
<UiLoadingSpinner className="size-6 text-muted-foreground" />
|
||||
<p className="text-sm">Loading tools from spec...</p>
|
||||
<p className="text-sm">Loading tools...</p>
|
||||
</div>
|
||||
)}
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue