mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-14 23:21:35 +00:00
feat(ui): simplify DCR connect grid, auto-finish on tab close, fix connect-status flash
In the gateway DCR connect flow the apps grid now reads as "authorize your servers" rather than a chat feature; the connectMode prop drops the Beta badge, the "use in chat" subtitle, and the tool-count chrome Closing the connect tab now best-effort finishes the flow via navigator.sendBeacon to /authorize/complete, so the gateway authorization code still reaches the client's loopback without an explicit click; the explicit "Finish connecting" button stays as the reliable path. The beacon is skipped while a per-server authorize is navigating away and after the button was pressed, so it never double-delivers or fires mid-authorize Authorized servers previously flashed "Connect" for a second before flipping to "Connected" because the per-user credential checks ran only after the whole tool-count fetch finished. They now fire in parallel with the tool-count load, and each card shows a skeleton in the button slot until its status resolves, so the state never flips under the user
This commit is contained in:
parent
262e93ec27
commit
5ab7a855c3
7 changed files with 155 additions and 52 deletions
|
|
@ -1712,7 +1712,7 @@
|
|||
},
|
||||
"src/components/chat/MCPAppsPanel.tsx": {
|
||||
"no-nested-ternary": {
|
||||
"count": 7
|
||||
"count": 6
|
||||
}
|
||||
},
|
||||
"src/components/chat/MCPConnectPicker.tsx": {
|
||||
|
|
|
|||
|
|
@ -33,7 +33,12 @@ function IntegrationsPageContent() {
|
|||
return (
|
||||
<div className="flex-1 min-h-0 overflow-auto w-full py-8 px-8">
|
||||
{connectFlow && <ConnectFlowBanner flowHandle={connectFlow} clientOrigin={connectClient} />}
|
||||
<MCPAppsPanel accessToken={accessToken} selectedServers={selectedMCPServers} onChange={setSelectedMCPServers} />
|
||||
<MCPAppsPanel
|
||||
accessToken={accessToken}
|
||||
selectedServers={selectedMCPServers}
|
||||
onChange={setSelectedMCPServers}
|
||||
connectMode={!!connectFlow}
|
||||
/>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,11 +1,17 @@
|
|||
import { describe, expect, it, vi } from "vitest";
|
||||
import { render, screen } from "@testing-library/react";
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import { fireEvent, render, screen } from "@testing-library/react";
|
||||
import ConnectFlowBanner from "./ConnectFlowBanner";
|
||||
import { PERSERVER_CONNECTING_KEY } from "@/hooks/mcpOAuthUtils";
|
||||
|
||||
vi.mock("@/components/networking", () => ({
|
||||
getProxyBaseUrl: () => "https://gateway.example.com",
|
||||
}));
|
||||
|
||||
afterEach(() => {
|
||||
vi.restoreAllMocks();
|
||||
sessionStorage.clear();
|
||||
});
|
||||
|
||||
describe("ConnectFlowBanner", () => {
|
||||
it("posts the flow handle to the proxy /authorize/complete as a full-page form", () => {
|
||||
const { container } = render(<ConnectFlowBanner flowHandle="flow-handle-123" clientOrigin="https://claude.ai" />);
|
||||
|
|
@ -30,4 +36,41 @@ describe("ConnectFlowBanner", () => {
|
|||
render(<ConnectFlowBanner flowHandle="h" clientOrigin={null} />);
|
||||
expect(screen.getAllByText(/the application/).length).toBeGreaterThan(0);
|
||||
});
|
||||
|
||||
it("best-effort auto-finishes on pagehide (closing the tab)", () => {
|
||||
const beaconMock = vi.fn(() => true);
|
||||
vi.stubGlobal("navigator", { ...navigator, sendBeacon: beaconMock });
|
||||
render(<ConnectFlowBanner flowHandle="flow-xyz" clientOrigin="https://claude.ai" />);
|
||||
|
||||
window.dispatchEvent(new Event("pagehide"));
|
||||
|
||||
expect(beaconMock).toHaveBeenCalledTimes(1);
|
||||
const [url, body] = beaconMock.mock.calls[0] as unknown as [string, URLSearchParams];
|
||||
expect(url).toBe("https://gateway.example.com/authorize/complete");
|
||||
expect(body.toString()).toContain("flow=flow-xyz");
|
||||
});
|
||||
|
||||
it("does NOT auto-finish while a per-server connect is navigating away", () => {
|
||||
const beaconMock = vi.fn(() => true);
|
||||
vi.stubGlobal("navigator", { ...navigator, sendBeacon: beaconMock });
|
||||
render(<ConnectFlowBanner flowHandle="flow-xyz" clientOrigin="https://claude.ai" />);
|
||||
|
||||
// the per-server connect flow sets this right before it navigates to the upstream IdP
|
||||
sessionStorage.setItem(PERSERVER_CONNECTING_KEY, "1");
|
||||
window.dispatchEvent(new Event("pagehide"));
|
||||
|
||||
expect(beaconMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("does NOT double-fire the auto-finish after the button was pressed", () => {
|
||||
const beaconMock = vi.fn(() => true);
|
||||
vi.stubGlobal("navigator", { ...navigator, sendBeacon: beaconMock });
|
||||
const { container } = render(<ConnectFlowBanner flowHandle="flow-xyz" clientOrigin="https://claude.ai" />);
|
||||
|
||||
// jsdom does not submit forms; fire the form's submit so onSubmit marks it finished
|
||||
fireEvent.submit(container.querySelector("form")!);
|
||||
window.dispatchEvent(new Event("pagehide"));
|
||||
|
||||
expect(beaconMock).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1,8 +1,9 @@
|
|||
"use client";
|
||||
|
||||
import React from "react";
|
||||
import React, { useEffect, useRef } from "react";
|
||||
import { CheckCircle } from "lucide-react";
|
||||
import { getProxyBaseUrl } from "@/components/networking";
|
||||
import { PERSERVER_CONNECTING_KEY } from "@/hooks/mcpOAuthUtils";
|
||||
|
||||
interface Props {
|
||||
flowHandle: string;
|
||||
|
|
@ -13,17 +14,38 @@ interface Props {
|
|||
* The interlude shown when a DCR client (Claude Desktop, MCP Inspector) sends the user
|
||||
* through the gateway sign-in and lands them on the apps grid to authorize servers. The
|
||||
* grid below authorizes individual servers into the per-user vault; this banner is the
|
||||
* deliberate finish step.
|
||||
* finish step that returns the user to the client.
|
||||
*
|
||||
* "Finish connecting" is a native form POST to the proxy's /authorize/complete, not a
|
||||
* fetch: the endpoint 303-redirects the browser back to the DCR client's own redirect URI
|
||||
* with the gateway authorization code, and only a full-page navigation carries the
|
||||
* HttpOnly per-flow cookie and follows that cross-origin redirect. The flow handle is the
|
||||
* only field; the sealed flow cookie set at /authorize holds everything else.
|
||||
* Finishing happens two ways, both hitting the proxy's /authorize/complete, which mints the
|
||||
* gateway authorization code and 303-redirects to the DCR client's own redirect URI:
|
||||
* - The explicit "Finish connecting" button is a native form POST, so the full-page
|
||||
* navigation carries the HttpOnly per-flow cookie and follows the cross-origin redirect
|
||||
* to the client's loopback. This is the reliable path.
|
||||
* - Closing (or navigating away from) the tab fires a best-effort navigator.sendBeacon to the
|
||||
* same endpoint. The browser follows the 303 to the client's loopback, so in most browsers
|
||||
* the code still reaches the client without an explicit click. This is a convenience, not a
|
||||
* consent gate: consent already happened at sign-in, so returning the user is safe. It is
|
||||
* skipped while a per-server connect is navigating away (that is not leaving the flow),
|
||||
* and after the button was pressed (which already delivers the code).
|
||||
*/
|
||||
const ConnectFlowBanner: React.FC<Props> = ({ flowHandle, clientOrigin }) => {
|
||||
const action = `${getProxyBaseUrl()}/authorize/complete`;
|
||||
const clientLabel = clientOrigin ?? "the application";
|
||||
const finishedRef = useRef(false);
|
||||
|
||||
useEffect(() => {
|
||||
sessionStorage.removeItem(PERSERVER_CONNECTING_KEY);
|
||||
|
||||
const autoFinishOnLeave = () => {
|
||||
if (finishedRef.current) return;
|
||||
if (sessionStorage.getItem(PERSERVER_CONNECTING_KEY) === "1") return;
|
||||
if (typeof navigator.sendBeacon === "function") {
|
||||
navigator.sendBeacon(action, new URLSearchParams({ flow: flowHandle }));
|
||||
}
|
||||
};
|
||||
window.addEventListener("pagehide", autoFinishOnLeave);
|
||||
return () => window.removeEventListener("pagehide", autoFinishOnLeave);
|
||||
}, [action, flowHandle]);
|
||||
|
||||
return (
|
||||
<div className="mb-6 rounded-lg border border-primary/30 bg-primary/5 px-5 py-4">
|
||||
|
|
@ -33,12 +55,12 @@ const ConnectFlowBanner: React.FC<Props> = ({ flowHandle, clientOrigin }) => {
|
|||
<div className="min-w-0">
|
||||
<p className="text-sm font-semibold text-foreground">Connect your MCP servers to {clientLabel}</p>
|
||||
<p className="text-[13px] text-muted-foreground mt-0.5">
|
||||
Authorize the servers you want to use below. When you are ready, finish connecting and you will be
|
||||
returned to {clientLabel}.
|
||||
Authorize the servers you want to use below, then finish connecting to return to {clientLabel}. Closing
|
||||
this tab finishes for you.
|
||||
</p>
|
||||
</div>
|
||||
</div>
|
||||
<form method="POST" action={action} className="shrink-0">
|
||||
<form method="POST" action={action} className="shrink-0" onSubmit={() => (finishedRef.current = true)}>
|
||||
<input type="hidden" name="flow" value={flowHandle} />
|
||||
<button
|
||||
type="submit"
|
||||
|
|
|
|||
|
|
@ -70,6 +70,7 @@ interface Props {
|
|||
accessToken: string;
|
||||
selectedServers: string[];
|
||||
onChange: (servers: string[]) => void;
|
||||
connectMode?: boolean;
|
||||
}
|
||||
|
||||
const AVATAR_COLORS = [
|
||||
|
|
@ -95,7 +96,7 @@ type TabKey = "all" | "connected";
|
|||
|
||||
const TOOLS_FETCH_CONCURRENCY = 5;
|
||||
|
||||
const MCPAppsPanel: React.FC<Props> = ({ accessToken, selectedServers, onChange }) => {
|
||||
const MCPAppsPanel: React.FC<Props> = ({ accessToken, selectedServers, onChange, connectMode }) => {
|
||||
const [servers, setServers] = useState<MCPServer[]>([]);
|
||||
const [loading, setLoading] = useState(true);
|
||||
const [query, setQuery] = useState("");
|
||||
|
|
@ -105,6 +106,7 @@ const MCPAppsPanel: React.FC<Props> = ({ accessToken, selectedServers, onChange
|
|||
const [toolCounts, setToolCounts] = useState<Record<string, number>>({});
|
||||
const [loadingCounts, setLoadingCounts] = useState(false);
|
||||
const [oauthConnected, setOauthConnected] = useState<Set<string>>(new Set());
|
||||
const [oauthChecking, setOauthChecking] = useState<Set<string>>(new Set());
|
||||
|
||||
const serversRef = useRef<MCPServer[]>([]);
|
||||
useEffect(() => {
|
||||
|
|
@ -147,6 +149,14 @@ const MCPAppsPanel: React.FC<Props> = ({ accessToken, selectedServers, onChange
|
|||
}
|
||||
} catch {
|
||||
// ignore
|
||||
} finally {
|
||||
if (!fetchLoadCancelledRef.current) {
|
||||
setOauthChecking((prev) => {
|
||||
const next = new Set(prev);
|
||||
next.delete(server.server_id);
|
||||
return next;
|
||||
});
|
||||
}
|
||||
}
|
||||
},
|
||||
[accessToken],
|
||||
|
|
@ -159,9 +169,13 @@ const MCPAppsPanel: React.FC<Props> = ({ accessToken, selectedServers, onChange
|
|||
.then(async (serverData) => {
|
||||
if (fetchLoadCancelledRef.current) return;
|
||||
const list: MCPServer[] = Array.isArray(serverData) ? serverData : serverData?.data ?? [];
|
||||
const oauthServers = list.filter((s) => s.auth_type === AUTH_TYPE.OAUTH2);
|
||||
setServers(list);
|
||||
setOauthChecking(new Set(oauthServers.map((s) => s.server_id)));
|
||||
setLoading(false);
|
||||
|
||||
oauthServers.forEach((s) => checkOauthCredential(s));
|
||||
|
||||
setLoadingCounts(true);
|
||||
const chunks = Array.from({ length: Math.ceil(list.length / TOOLS_FETCH_CONCURRENCY) }, (_, i) =>
|
||||
list.slice(i * TOOLS_FETCH_CONCURRENCY, (i + 1) * TOOLS_FETCH_CONCURRENCY),
|
||||
|
|
@ -171,9 +185,6 @@ const MCPAppsPanel: React.FC<Props> = ({ accessToken, selectedServers, onChange
|
|||
await Promise.allSettled(chunk.map((s) => fetchToolCount(s)));
|
||||
}
|
||||
if (!fetchLoadCancelledRef.current) setLoadingCounts(false);
|
||||
|
||||
const oauthServers = list.filter((s) => s.auth_type === AUTH_TYPE.OAUTH2);
|
||||
oauthServers.forEach((s) => checkOauthCredential(s));
|
||||
})
|
||||
.catch(() => {
|
||||
if (!fetchLoadCancelledRef.current) {
|
||||
|
|
@ -230,6 +241,29 @@ const MCPAppsPanel: React.FC<Props> = ({ accessToken, selectedServers, onChange
|
|||
}
|
||||
};
|
||||
|
||||
const renderConnectionIndicator = (server: MCPServer) => {
|
||||
if (server.auth_type === AUTH_TYPE.OAUTH2) {
|
||||
if (oauthConnected.has(server.server_id)) {
|
||||
return <CheckCircle className="h-3.5 w-3.5 text-emerald-600 shrink-0" />;
|
||||
}
|
||||
if (oauthChecking.has(server.server_id)) {
|
||||
return <Skeleton className="h-6 w-16 shrink-0 rounded-md" />;
|
||||
}
|
||||
return (
|
||||
<OAuth2ConnectButton
|
||||
server={server}
|
||||
accessToken={accessToken}
|
||||
onConnect={(id) => setOauthConnected((prev) => new Set(prev).add(id))}
|
||||
variant="badge"
|
||||
/>
|
||||
);
|
||||
}
|
||||
if (selectedServers.includes(nameOf(server))) {
|
||||
return <span className="w-[7px] h-[7px] rounded-full bg-emerald-600 dark:bg-emerald-400 shrink-0" />;
|
||||
}
|
||||
return null;
|
||||
};
|
||||
|
||||
const { data: detailToolsResult, isLoading: loadingTools } = useQuery({
|
||||
queryKey: ["mcp-apps-panel-detail-tools", detailServer?.server_id],
|
||||
queryFn: () => listMCPTools(accessToken, detailServer!.server_id),
|
||||
|
|
@ -396,24 +430,30 @@ const MCPAppsPanel: React.FC<Props> = ({ accessToken, selectedServers, onChange
|
|||
<div>
|
||||
<div className="flex items-center gap-2 mb-1">
|
||||
<h2 className="m-0 text-lg font-semibold text-foreground">MCP Servers</h2>
|
||||
<span className="text-[10px] font-semibold text-primary bg-primary/10 rounded px-1.5 py-0.5 uppercase tracking-wider">
|
||||
Beta
|
||||
</span>
|
||||
</div>
|
||||
<div className="flex items-center gap-3">
|
||||
<p className="m-0 text-[13px] text-muted-foreground">Browse tools, authenticate once, use in chat</p>
|
||||
{loadingCounts ? (
|
||||
<span className="flex items-center gap-1.5 text-xs text-muted-foreground">
|
||||
<Loader2 className="h-3 w-3 animate-spin" />
|
||||
Loading tools...
|
||||
{!connectMode && (
|
||||
<span className="text-[10px] font-semibold text-primary bg-primary/10 rounded px-1.5 py-0.5 uppercase tracking-wider">
|
||||
Beta
|
||||
</span>
|
||||
) : totalTools > 0 ? (
|
||||
<span className="flex items-center gap-1 text-xs text-muted-foreground">
|
||||
<Wrench className="h-3 w-3" />
|
||||
{totalTools} tool{totalTools !== 1 ? "s" : ""} available
|
||||
</span>
|
||||
) : null}
|
||||
)}
|
||||
</div>
|
||||
{connectMode ? (
|
||||
<p className="m-0 text-[13px] text-muted-foreground">Click a server to see its tools and connect</p>
|
||||
) : (
|
||||
<div className="flex items-center gap-3">
|
||||
<p className="m-0 text-[13px] text-muted-foreground">Browse tools, authenticate once, use in chat</p>
|
||||
{loadingCounts ? (
|
||||
<span className="flex items-center gap-1.5 text-xs text-muted-foreground">
|
||||
<Loader2 className="h-3 w-3 animate-spin" />
|
||||
Loading tools...
|
||||
</span>
|
||||
) : totalTools > 0 ? (
|
||||
<span className="flex items-center gap-1 text-xs text-muted-foreground">
|
||||
<Wrench className="h-3 w-3" />
|
||||
{totalTools} tool{totalTools !== 1 ? "s" : ""} available
|
||||
</span>
|
||||
) : null}
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
<div className="relative w-[220px]">
|
||||
<Search className="absolute left-3 top-1/2 -translate-y-1/2 h-3.5 w-3.5 text-muted-foreground" />
|
||||
|
|
@ -464,7 +504,6 @@ const MCPAppsPanel: React.FC<Props> = ({ accessToken, selectedServers, onChange
|
|||
<div className="grid grid-cols-2 border rounded-lg overflow-hidden">
|
||||
{filtered.map((server, idx) => {
|
||||
const name = nameOf(server);
|
||||
const isConnected = selectedServers.includes(name);
|
||||
const color = getAvatarColor(name);
|
||||
const isLeftCol = idx % 2 === 0;
|
||||
const count = toolCounts[name];
|
||||
|
|
@ -513,22 +552,7 @@ const MCPAppsPanel: React.FC<Props> = ({ accessToken, selectedServers, onChange
|
|||
) : null}
|
||||
</div>
|
||||
</div>
|
||||
{server.auth_type === AUTH_TYPE.OAUTH2 ? (
|
||||
oauthConnected.has(server.server_id) ? (
|
||||
<CheckCircle className="h-3.5 w-3.5 text-emerald-600 shrink-0" />
|
||||
) : (
|
||||
<OAuth2ConnectButton
|
||||
server={server}
|
||||
accessToken={accessToken}
|
||||
onConnect={(id) => {
|
||||
setOauthConnected((prev) => new Set(prev).add(id));
|
||||
}}
|
||||
variant="badge"
|
||||
/>
|
||||
)
|
||||
) : isConnected ? (
|
||||
<span className="w-[7px] h-[7px] rounded-full bg-emerald-600 dark:bg-emerald-400 shrink-0" />
|
||||
) : null}
|
||||
{renderConnectionIndicator(server)}
|
||||
<ChevronRight className="h-3 w-3 text-muted-foreground/40 shrink-0" />
|
||||
</div>
|
||||
);
|
||||
|
|
|
|||
|
|
@ -16,6 +16,14 @@ import { getProxyBaseUrl, serverRootPath } from "@/components/networking";
|
|||
*/
|
||||
export const TOOLS_OAUTH_UI_STATE_KEY = "litellm-mcp-oauth-tools-state";
|
||||
|
||||
/**
|
||||
* sessionStorage flag set by useUserMcpOAuthFlow right before it navigates the whole page
|
||||
* to the upstream IdP to authorize one server. ConnectFlowBanner's auto-finish-on-close
|
||||
* handler skips while this is set, so authorizing a server is not mistaken for the user
|
||||
* leaving the gateway DCR connect flow.
|
||||
*/
|
||||
export const PERSERVER_CONNECTING_KEY = "litellm-mcp-perserver-connecting";
|
||||
|
||||
/**
|
||||
* Build the OAuth callback URL for the current UI deployment.
|
||||
*
|
||||
|
|
|
|||
|
|
@ -23,7 +23,7 @@ import NotificationsManager from "@/components/molecules/notifications_manager";
|
|||
import { extractErrorMessage } from "@/utils/errorUtils";
|
||||
import { generateCodeChallenge, generateCodeVerifier } from "@/utils/pkce";
|
||||
import { getSecureItem, setSecureItem } from "@/utils/secureStorage";
|
||||
import { buildCallbackUrl, clearStorage } from "./mcpOAuthUtils";
|
||||
import { buildCallbackUrl, clearStorage, PERSERVER_CONNECTING_KEY } from "./mcpOAuthUtils";
|
||||
|
||||
export type UserMcpOAuthStatus = "idle" | "authorizing" | "exchanging" | "success" | "error";
|
||||
|
||||
|
|
@ -135,6 +135,7 @@ export const useUserMcpOAuthFlow = ({
|
|||
returnUrl.searchParams.set("mcpOauthReturn", "apps");
|
||||
setStorage(RETURN_URL_KEY, returnUrl.toString());
|
||||
|
||||
sessionStorage.setItem(PERSERVER_CONNECTING_KEY, "1");
|
||||
window.location.href = authorizeUrl;
|
||||
} catch (err) {
|
||||
const msg = extractErrorMessage(err);
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue