mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-11 22:51:28 +00:00
Merge pull request #40498 from BerriAI/litellm_fix_mcp_edit_tool_preview_7135
fix(mcp): refresh tool previews when editing connection settings
This commit is contained in:
joshua-berri
GitHub
commit
083ddefa92
7 changed files with 472 additions and 18 deletions
|
|
@ -1234,7 +1234,7 @@ if MCP_AVAILABLE:
|
|||
return client_id, client_secret, scopes
|
||||
|
||||
_STAGED_AUTH_VALUE_AUTH_TYPES: Final = frozenset(
|
||||
(MCPAuth.api_key, MCPAuth.bearer_token, MCPAuth.basic, MCPAuth.authorization)
|
||||
(MCPAuth.api_key, MCPAuth.bearer_token, MCPAuth.basic, MCPAuth.authorization, MCPAuth.token)
|
||||
)
|
||||
|
||||
@dataclass(frozen=True, slots=True)
|
||||
|
|
@ -1243,6 +1243,17 @@ if MCP_AVAILABLE:
|
|||
mcp_auth_header: str | None
|
||||
oauth2_headers: dict[str, str] | None
|
||||
|
||||
def _preview_origin(url: str | None) -> tuple[str, str, int | None] | None:
|
||||
if not url:
|
||||
return None
|
||||
try:
|
||||
parsed: Final = httpx.URL(url)
|
||||
except httpx.InvalidURL:
|
||||
return None
|
||||
if parsed.scheme not in ("http", "https") or not parsed.host:
|
||||
return None
|
||||
return parsed.scheme, parsed.host, parsed.port
|
||||
|
||||
def _stage_server_test(new_mcp_server_request: NewMCPServerRequest, headers: Headers) -> _StagedServerTest:
|
||||
"""
|
||||
Resolve the credentials a not-yet-saved server config carries for a preview call.
|
||||
|
|
@ -1255,7 +1266,19 @@ if MCP_AVAILABLE:
|
|||
MCPRequestHandler,
|
||||
)
|
||||
|
||||
request: Final = _inherit_credentials_from_existing_server(new_mcp_server_request)
|
||||
saved_server: Final = (
|
||||
global_mcp_server_manager.get_mcp_server_by_id(new_mcp_server_request.server_id)
|
||||
if new_mcp_server_request.server_id
|
||||
else None
|
||||
)
|
||||
saved_origin: Final = _preview_origin(saved_server.url) if saved_server else None
|
||||
preview_origin: Final = _preview_origin(new_mcp_server_request.url)
|
||||
may_inherit: Final = new_mcp_server_request.auth_type not in _STAGED_AUTH_VALUE_AUTH_TYPES or (
|
||||
saved_origin is not None and saved_origin == preview_origin
|
||||
)
|
||||
request: Final = (
|
||||
_inherit_credentials_from_existing_server(new_mcp_server_request) if may_inherit else new_mcp_server_request
|
||||
)
|
||||
mcp_auth_header: Final = (
|
||||
request.credentials.get("auth_value")
|
||||
if request.auth_type in _STAGED_AUTH_VALUE_AUTH_TYPES and isinstance(request.credentials, dict)
|
||||
|
|
@ -1318,8 +1341,15 @@ if MCP_AVAILABLE:
|
|||
if _oauth2_flow == "client_credentials" and not request.token_url:
|
||||
_oauth2_flow = None
|
||||
|
||||
# Static previews inherit credentials before this step, but must not resolve back to
|
||||
# the saved record during client creation and discard the edited connection settings.
|
||||
preview_server_id: Final = (
|
||||
""
|
||||
if request.auth_type in _STAGED_AUTH_VALUE_AUTH_TYPES or request.auth_type in (None, MCPAuth.none)
|
||||
else request.server_id or ""
|
||||
)
|
||||
server_model: Final = MCPServer(
|
||||
server_id=request.server_id or "",
|
||||
server_id=preview_server_id,
|
||||
name=request.alias or request.server_name or "",
|
||||
url=request.url,
|
||||
transport=request.transport,
|
||||
|
|
|
|||
|
|
@ -87,6 +87,125 @@ def _route_has_dependency(route, dependency) -> bool:
|
|||
|
||||
|
||||
class TestExecuteWithMcpClient:
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.parametrize(
|
||||
("auth_type", "auth_value", "expected_auth"),
|
||||
(
|
||||
(MCPAuth.none, None, {}),
|
||||
(MCPAuth.basic, "preview:correct", {"Authorization": "Basic cHJldmlldzpjb3JyZWN0"}),
|
||||
(MCPAuth.basic, None, {"Authorization": "Basic cHJldmlldzpzdG9yZWQ="}),
|
||||
(MCPAuth.bearer_token, "edited", {"Authorization": "Bearer edited"}),
|
||||
(MCPAuth.api_key, "edited", {"X-API-Key": "edited"}),
|
||||
(MCPAuth.token, "edited", {"Authorization": "token edited"}),
|
||||
(MCPAuth.authorization, "Custom edited", {"Authorization": "Custom edited"}),
|
||||
),
|
||||
)
|
||||
async def test_static_preview_uses_edited_connection_instead_of_registered_server(
|
||||
self,
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
auth_type: MCPAuth,
|
||||
auth_value: str | None,
|
||||
expected_auth: dict[str, str],
|
||||
) -> None:
|
||||
from starlette.datastructures import Headers
|
||||
|
||||
from litellm.experimental_mcp_client.client import MCPClient
|
||||
from litellm.proxy._experimental.mcp_server.mcp_server_manager import MCPServerManager
|
||||
from litellm.proxy.management_endpoints import mcp_management_endpoints
|
||||
|
||||
saved: Final = MCPServer(
|
||||
server_id="saved-preview-server",
|
||||
name="saved",
|
||||
url="https://stored.example/mcp",
|
||||
transport=MCPTransport.http,
|
||||
auth_type=MCPAuth.basic,
|
||||
authentication_token="preview:stored",
|
||||
)
|
||||
manager: Final = MCPServerManager()
|
||||
manager.registry = {saved.server_id: saved}
|
||||
monkeypatch.setattr(rest_endpoints, "global_mcp_server_manager", manager)
|
||||
monkeypatch.setattr(mcp_management_endpoints, "global_mcp_server_manager", manager)
|
||||
payload: Final = NewMCPServerRequest(
|
||||
server_id=saved.server_id,
|
||||
server_name="edited",
|
||||
url="https://stored.example/corrected-mcp",
|
||||
transport=MCPTransport.sse,
|
||||
auth_type=auth_type,
|
||||
credentials={"auth_value": auth_value} if auth_value is not None else None,
|
||||
static_headers={"X-Preview": "edited"},
|
||||
)
|
||||
staged: Final = rest_endpoints._stage_server_test(payload, Headers())
|
||||
|
||||
async def inspect_connection(client: MCPClient) -> dict[str, object]:
|
||||
return {"url": client.server_url, "transport": client.transport_type, "headers": client._get_auth_headers()}
|
||||
|
||||
result: Final = await rest_endpoints._execute_with_mcp_client(
|
||||
staged.request,
|
||||
inspect_connection,
|
||||
mcp_auth_header=staged.mcp_auth_header,
|
||||
oauth2_headers=staged.oauth2_headers,
|
||||
)
|
||||
assert result == {
|
||||
"url": "https://stored.example/corrected-mcp",
|
||||
"transport": MCPTransport.sse,
|
||||
"headers": {"X-Preview": "edited", **expected_auth},
|
||||
}
|
||||
assert manager.get_mcp_server_by_id(saved.server_id) is saved
|
||||
assert saved.url == "https://stored.example/mcp"
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("saved_url", "url", "same_origin"),
|
||||
(
|
||||
("https://stored.example/mcp", "https://other.example/mcp", False),
|
||||
("https://stored.example/mcp", "http://stored.example/mcp", False),
|
||||
("https://stored.example/mcp", "https://stored.example:8443/mcp", False),
|
||||
("https://stored.example/mcp", "https://stored.example:443/mcp", True),
|
||||
("http://stored.example/mcp", "http://stored.example:80/edited", True),
|
||||
("https://stored.example/mcp", "HTTPS://STORED.EXAMPLE/edited", True),
|
||||
("https://[::1]/mcp", "https://[::1]/edited", True),
|
||||
("https://[::1]/mcp", "https://[::1]:443/edited", True),
|
||||
("https://[::1]/mcp", "https://[::2]/edited", False),
|
||||
("https://stored.example/mcp", "https://stored.example:invalid/mcp", False),
|
||||
),
|
||||
)
|
||||
@pytest.mark.parametrize("explicit_credential", (None, "preview:explicit"))
|
||||
def test_static_preview_respects_origin_when_inheriting_credentials(
|
||||
self,
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
saved_url: str,
|
||||
url: str,
|
||||
same_origin: bool,
|
||||
explicit_credential: str | None,
|
||||
) -> None:
|
||||
from starlette.datastructures import Headers
|
||||
|
||||
from litellm.proxy._experimental.mcp_server.mcp_server_manager import MCPServerManager
|
||||
from litellm.proxy.management_endpoints import mcp_management_endpoints
|
||||
|
||||
saved: Final = MCPServer(
|
||||
server_id="saved-preview-server",
|
||||
name="saved",
|
||||
url=saved_url,
|
||||
transport=MCPTransport.http,
|
||||
auth_type=MCPAuth.basic,
|
||||
authentication_token="preview:stored",
|
||||
)
|
||||
manager: Final = MCPServerManager()
|
||||
manager.registry = {saved.server_id: saved}
|
||||
monkeypatch.setattr(rest_endpoints, "global_mcp_server_manager", manager)
|
||||
monkeypatch.setattr(mcp_management_endpoints, "global_mcp_server_manager", manager)
|
||||
payload: Final = NewMCPServerRequest(
|
||||
server_id=saved.server_id,
|
||||
url=url,
|
||||
transport=MCPTransport.http,
|
||||
auth_type=MCPAuth.basic,
|
||||
credentials={"auth_value": explicit_credential} if explicit_credential else None,
|
||||
)
|
||||
staged: Final = rest_endpoints._stage_server_test(payload, Headers())
|
||||
expected: Final = explicit_credential or ("preview:stored" if same_origin else None)
|
||||
assert staged.mcp_auth_header == expected
|
||||
assert staged.request.credentials == ({"auth_value": expected} if expected else None)
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_redacts_stack_trace(self, monkeypatch):
|
||||
async def fake_create_client(*args, **kwargs):
|
||||
|
|
|
|||
|
|
@ -0,0 +1,105 @@
|
|||
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://example.com/corrected-mcp" }, saved)).toEqual({
|
||||
kind: "preview",
|
||||
config: {
|
||||
url: "https://example.com/corrected-mcp",
|
||||
transport: "http",
|
||||
auth_type: "basic",
|
||||
static_headers: { "X-Tenant": "original" },
|
||||
credentials: undefined,
|
||||
},
|
||||
});
|
||||
});
|
||||
|
||||
it.each(["https://other.example/mcp", "http://example.com/mcp", "https://example.com:8443/mcp"])(
|
||||
"requires explicit credentials for a changed origin: %s",
|
||||
(url) => {
|
||||
expect(getEditToolPreview({ ...saved, url, static_headers: [] }, saved)).toEqual({
|
||||
kind: "incomplete",
|
||||
message: expect.stringContaining("origin changed"),
|
||||
});
|
||||
const explicit = { ...saved, url, static_headers: [], credentials: { auth_value: "new:secret" } };
|
||||
expect(getEditToolPreview(explicit, saved).kind).toBe("preview");
|
||||
},
|
||||
);
|
||||
|
||||
it("does not automatically send saved static headers to a new origin", () => {
|
||||
expect(getEditToolPreview({ ...saved, url: "https://other.example/mcp", auth_type: "none" }, saved).kind).toBe(
|
||||
"incomplete",
|
||||
);
|
||||
});
|
||||
|
||||
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,67 @@
|
|||
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 message?: string }
|
||||
| { 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" };
|
||||
}
|
||||
const savedConfig = connectionConfig(initialValues);
|
||||
const changedOrigin =
|
||||
!URL.canParse(savedConfig.url) || new URL(config.url).origin !== new URL(savedConfig.url).origin;
|
||||
const reusesHeader = Object.entries(config.static_headers).some(
|
||||
([key, value]) => savedConfig.static_headers[key] === value,
|
||||
);
|
||||
const needsSavedCredential = AUTH_TYPES_REQUIRING_AUTH_VALUE.includes(config.auth_type) && !config.credentials;
|
||||
if (changedOrigin && (needsSavedCredential || reusesHeader)) {
|
||||
return {
|
||||
kind: "incomplete",
|
||||
message:
|
||||
"The server origin changed. Enter credentials and replace or remove saved static headers to preview tools.",
|
||||
};
|
||||
}
|
||||
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,30 @@ 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() === "") {
|
||||
const controller = new AbortController();
|
||||
setTools([]);
|
||||
setToolsError(null);
|
||||
setIsLoadingTools(false);
|
||||
if (!accessToken || !mcpServer.server_id) return;
|
||||
if (toolPreview.kind === "incomplete") {
|
||||
setToolsError(toolPreview.message ?? "Complete the URL, authentication, and header settings to load tools.");
|
||||
return;
|
||||
}
|
||||
fetchTools();
|
||||
setIsLoadingTools(true);
|
||||
const timer = setTimeout(
|
||||
() => fetchTools(() => !controller.signal.aborted),
|
||||
toolPreview.kind === "preview" ? 500 : 0,
|
||||
);
|
||||
return () => {
|
||||
controller.abort();
|
||||
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 +536,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 +568,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 +576,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 +594,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 +602,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 +613,7 @@ const MCPServerEdit: React.FC<MCPServerEditProps> = ({
|
|||
? getToken(mcpServer.server_id, userID)?.access_token ?? null
|
||||
: null);
|
||||
if (!token) {
|
||||
setIsLoadingTools(false);
|
||||
setTools([]);
|
||||
setToolsError(
|
||||
isBrowserHeldTokenMode
|
||||
|
|
@ -608,7 +631,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 +648,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