mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-09 03:18:44 +00:00
fix(ui): show info message when MCP tool preview returns 403
Internal users submitting MCP servers hit an admin-only preview endpoint; replace the red connection error with a clear review notice while leaving other failures unchanged. Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
parent
13b590c8ec
commit
e45f36e4ef
7 changed files with 68 additions and 13 deletions
|
|
@ -1,2 +1,5 @@
|
|||
// Must match the backend SpecialMCPServerNames.no_mcp_servers enum value.
|
||||
export const NO_MCP_SERVERS_SENTINEL = "no-mcp-servers";
|
||||
|
||||
export const MCP_TOOLS_PREVIEW_FORBIDDEN_MESSAGE =
|
||||
"Tool preview is not available for submissions. Tools will be verified by an admin during review.";
|
||||
|
|
|
|||
|
|
@ -91,7 +91,7 @@ const CreateMCPServer: React.FC<CreateMCPServerProps> = ({
|
|||
const [oauthDocsUrl, setOauthDocsUrl] = useState<string | null>(null);
|
||||
|
||||
// Single hook call shared by MCPConnectionStatus and MCPToolConfiguration to avoid duplicate requests.
|
||||
const { tools, isLoadingTools, toolsError, toolsErrorStackTrace, canFetchTools, fetchTools, clearTools } =
|
||||
const { tools, isLoadingTools, toolsError, toolsErrorStatus, toolsErrorStackTrace, canFetchTools, fetchTools, clearTools } =
|
||||
useTestMCPConnection({
|
||||
accessToken,
|
||||
oauthAccessToken,
|
||||
|
|
@ -1088,6 +1088,7 @@ const CreateMCPServer: React.FC<CreateMCPServerProps> = ({
|
|||
tools={tools}
|
||||
isLoadingTools={isLoadingTools}
|
||||
toolsError={toolsError}
|
||||
toolsErrorStatus={toolsErrorStatus}
|
||||
toolsErrorStackTrace={toolsErrorStackTrace}
|
||||
canFetchTools={canFetchTools}
|
||||
fetchTools={fetchTools}
|
||||
|
|
@ -1112,6 +1113,7 @@ const CreateMCPServer: React.FC<CreateMCPServerProps> = ({
|
|||
externalTools={tools}
|
||||
externalIsLoading={isLoadingTools}
|
||||
externalError={toolsError}
|
||||
externalErrorStatus={toolsErrorStatus}
|
||||
externalCanFetch={canFetchTools}
|
||||
/>
|
||||
</div>
|
||||
|
|
|
|||
|
|
@ -41,6 +41,21 @@ describe("MCPConnectionStatus", () => {
|
|||
expect(screen.getByText("Connecting...")).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("should show info message without retry when tool preview returns 403", () => {
|
||||
render(
|
||||
<MCPConnectionStatus
|
||||
{...defaultProps}
|
||||
canFetchTools={true}
|
||||
toolsError="Tool preview is not available for submissions. Tools will be verified by an admin during review."
|
||||
toolsErrorStatus={403}
|
||||
/>,
|
||||
);
|
||||
|
||||
expect(screen.getByRole("alert")).toHaveTextContent(/Tools will be verified by an admin during review/i);
|
||||
expect(screen.queryByText("Connection Failed")).not.toBeInTheDocument();
|
||||
expect(screen.queryByRole("button", { name: /retry/i })).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("should show error state with retry button when toolsError is set", async () => {
|
||||
const fetchTools = vi.fn();
|
||||
const user = userEvent.setup();
|
||||
|
|
|
|||
|
|
@ -8,6 +8,7 @@ interface MCPConnectionStatusProps {
|
|||
tools: any[];
|
||||
isLoadingTools: boolean;
|
||||
toolsError: string | null;
|
||||
toolsErrorStatus?: number | null;
|
||||
toolsErrorStackTrace: string | null;
|
||||
canFetchTools: boolean;
|
||||
fetchTools: () => Promise<void>;
|
||||
|
|
@ -18,10 +19,12 @@ const MCPConnectionStatus: React.FC<MCPConnectionStatusProps> = ({
|
|||
tools,
|
||||
isLoadingTools,
|
||||
toolsError,
|
||||
toolsErrorStatus = null,
|
||||
toolsErrorStackTrace,
|
||||
canFetchTools,
|
||||
fetchTools,
|
||||
}) => {
|
||||
const isPreviewForbidden = toolsErrorStatus === 403;
|
||||
// Don't show anything if required fields aren't filled
|
||||
if (!canFetchTools && !formValues.url && !formValues.spec_path) {
|
||||
return null;
|
||||
|
|
@ -54,7 +57,9 @@ const MCPConnectionStatus: React.FC<MCPConnectionStatusProps> = ({
|
|||
: tools.length > 0
|
||||
? "Connection successful"
|
||||
: toolsError
|
||||
? "Connection failed"
|
||||
? isPreviewForbidden
|
||||
? "Ready to submit"
|
||||
: "Connection failed"
|
||||
: "Ready to test connection"}
|
||||
</Text>
|
||||
<br />
|
||||
|
|
@ -75,7 +80,7 @@ const MCPConnectionStatus: React.FC<MCPConnectionStatusProps> = ({
|
|||
</div>
|
||||
)}
|
||||
|
||||
{toolsError && (
|
||||
{toolsError && !isPreviewForbidden && (
|
||||
<div className="flex items-center text-red-600">
|
||||
<ExclamationCircleOutlined className="mr-1" />
|
||||
<Text className="text-red-600 font-medium">Failed</Text>
|
||||
|
|
@ -90,7 +95,11 @@ const MCPConnectionStatus: React.FC<MCPConnectionStatusProps> = ({
|
|||
</div>
|
||||
)}
|
||||
|
||||
{toolsError && (
|
||||
{toolsError && isPreviewForbidden && (
|
||||
<Alert message="Tool preview unavailable" description={toolsError} type="info" showIcon />
|
||||
)}
|
||||
|
||||
{toolsError && !isPreviewForbidden && (
|
||||
<Alert
|
||||
message="Connection Failed"
|
||||
description={
|
||||
|
|
|
|||
|
|
@ -27,6 +27,7 @@ interface MCPToolConfigurationProps {
|
|||
externalTools?: any[];
|
||||
externalIsLoading?: boolean;
|
||||
externalError?: string | null;
|
||||
externalErrorStatus?: number | null;
|
||||
externalCanFetch?: boolean;
|
||||
/** When true, do not auto-select all tools for servers with no stored allowlist. */
|
||||
isEditMode?: boolean;
|
||||
|
|
@ -156,6 +157,7 @@ const MCPToolConfiguration: React.FC<MCPToolConfigurationProps> = ({
|
|||
externalTools,
|
||||
externalIsLoading,
|
||||
externalError,
|
||||
externalErrorStatus = null,
|
||||
externalCanFetch,
|
||||
isEditMode = false,
|
||||
}) => {
|
||||
|
|
@ -165,6 +167,7 @@ const MCPToolConfiguration: React.FC<MCPToolConfigurationProps> = ({
|
|||
const hasInitializedRef = useRef(false);
|
||||
const previousSuggestedToolNamesRef = useRef<string>("");
|
||||
const [expandedTools, setExpandedTools] = useState<Set<string>>(new Set());
|
||||
const isPreviewForbidden = externalErrorStatus === 403;
|
||||
|
||||
// Tool list is fetched by the parent (create/edit flow) and passed in. This
|
||||
// component renders that state; it never fetches on its own, so there is a
|
||||
|
|
@ -429,7 +432,13 @@ const MCPToolConfiguration: React.FC<MCPToolConfigurationProps> = ({
|
|||
)}
|
||||
|
||||
{/* Error state */}
|
||||
{toolsError && !isLoadingTools && (
|
||||
{toolsError && !isLoadingTools && isPreviewForbidden && (
|
||||
<div className="rounded-lg border border-blue-200 bg-blue-50 p-4">
|
||||
<Text className="text-sm text-blue-800">{toolsError}</Text>
|
||||
</div>
|
||||
)}
|
||||
|
||||
{toolsError && !isLoadingTools && !isPreviewForbidden && (
|
||||
<div className="text-center py-6 text-red-500 border rounded-lg border-dashed border-red-300 bg-red-50">
|
||||
<ToolOutlined className="text-2xl mb-2" />
|
||||
<Text className="text-red-600 font-medium">Unable to load tools</Text>
|
||||
|
|
|
|||
|
|
@ -31,6 +31,7 @@ import type { SkillRegisterRequest } from "./claude_code_plugins/types";
|
|||
import { jsonFields } from "./common_components/check_openapi_schema";
|
||||
import NotificationsManager from "./molecules/notifications_manager";
|
||||
import type { MCPUserEnvVarsStatus } from "./mcp_tools/types";
|
||||
import { MCP_TOOLS_PREVIEW_FORBIDDEN_MESSAGE } from "./mcp_tools/constants";
|
||||
import { createApiClient, deriveErrorMessage } from "@/lib/http/client";
|
||||
import { resolveApiBase } from "@/lib/http/resolveApiBase";
|
||||
import { serverRootPath, setServerRootPath } from "@/lib/serverRootPath";
|
||||
|
|
@ -6782,17 +6783,25 @@ export const testMCPToolsListRequest = async (
|
|||
const data = await response.json();
|
||||
|
||||
if (!response.ok || data.error) {
|
||||
if (response.status === 403) {
|
||||
return {
|
||||
tools: [],
|
||||
error: true,
|
||||
status: 403,
|
||||
message: MCP_TOOLS_PREVIEW_FORBIDDEN_MESSAGE,
|
||||
};
|
||||
}
|
||||
// Return the error response instead of throwing an error
|
||||
// This allows the caller to handle the error format properly
|
||||
if (data.error) {
|
||||
return data; // Return the full error response
|
||||
} else {
|
||||
return {
|
||||
tools: [],
|
||||
error: "request_failed",
|
||||
message: data.message || `MCP tools list failed: ${response.status} ${response.statusText}`,
|
||||
};
|
||||
return { ...data, status: response.status };
|
||||
}
|
||||
return {
|
||||
tools: [],
|
||||
error: "request_failed",
|
||||
status: response.status,
|
||||
message: data.message || `MCP tools list failed: ${response.status} ${response.statusText}`,
|
||||
};
|
||||
}
|
||||
|
||||
return data;
|
||||
|
|
|
|||
|
|
@ -33,6 +33,7 @@ interface UseTestMCPConnectionReturn {
|
|||
tools: any[];
|
||||
isLoadingTools: boolean;
|
||||
toolsError: string | null;
|
||||
toolsErrorStatus: number | null;
|
||||
toolsErrorStackTrace: string | null;
|
||||
hasShownSuccessMessage: boolean;
|
||||
canFetchTools: boolean;
|
||||
|
|
@ -49,6 +50,7 @@ export const useTestMCPConnection = ({
|
|||
const [tools, setTools] = useState<any[]>([]);
|
||||
const [isLoadingTools, setIsLoadingTools] = useState(false);
|
||||
const [toolsError, setToolsError] = useState<string | null>(null);
|
||||
const [toolsErrorStatus, setToolsErrorStatus] = useState<number | null>(null);
|
||||
const [toolsErrorStackTrace, setToolsErrorStackTrace] = useState<string | null>(null);
|
||||
const [hasShownSuccessMessage, setHasShownSuccessMessage] = useState(false);
|
||||
|
||||
|
|
@ -85,6 +87,7 @@ export const useTestMCPConnection = ({
|
|||
|
||||
setIsLoadingTools(true);
|
||||
setToolsError(null);
|
||||
setToolsErrorStatus(null);
|
||||
|
||||
try {
|
||||
// Prepare the MCP server config from form values
|
||||
|
|
@ -155,6 +158,7 @@ export const useTestMCPConnection = ({
|
|||
if (toolsResponse.tools && !toolsResponse.error) {
|
||||
setTools(toolsResponse.tools);
|
||||
setToolsError(null);
|
||||
setToolsErrorStatus(null);
|
||||
setToolsErrorStackTrace(null);
|
||||
if (toolsResponse.tools.length > 0 && !hasShownSuccessMessage) {
|
||||
setHasShownSuccessMessage(true);
|
||||
|
|
@ -162,13 +166,15 @@ export const useTestMCPConnection = ({
|
|||
} else {
|
||||
const errorMessage = toolsResponse.message || "Failed to retrieve tools list";
|
||||
setToolsError(errorMessage);
|
||||
setToolsErrorStackTrace(toolsResponse.stack_trace || null);
|
||||
setToolsErrorStatus(typeof toolsResponse.status === "number" ? toolsResponse.status : null);
|
||||
setToolsErrorStackTrace(toolsResponse.status === 403 ? null : toolsResponse.stack_trace || null);
|
||||
setTools([]);
|
||||
setHasShownSuccessMessage(false);
|
||||
}
|
||||
} catch (error) {
|
||||
console.error("Tools fetch error:", error);
|
||||
setToolsError(error instanceof Error ? error.message : String(error));
|
||||
setToolsErrorStatus(null);
|
||||
setToolsErrorStackTrace(null);
|
||||
setTools([]);
|
||||
setHasShownSuccessMessage(false);
|
||||
|
|
@ -180,6 +186,7 @@ export const useTestMCPConnection = ({
|
|||
const clearTools = useCallback(() => {
|
||||
setTools([]);
|
||||
setToolsError(null);
|
||||
setToolsErrorStatus(null);
|
||||
setToolsErrorStackTrace(null);
|
||||
setHasShownSuccessMessage(false);
|
||||
}, []);
|
||||
|
|
@ -213,6 +220,7 @@ export const useTestMCPConnection = ({
|
|||
tools,
|
||||
isLoadingTools,
|
||||
toolsError,
|
||||
toolsErrorStatus,
|
||||
toolsErrorStackTrace,
|
||||
hasShownSuccessMessage,
|
||||
canFetchTools,
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue