diff --git a/e2e/virtual-servers.spec.ts b/e2e/virtual-servers.spec.ts index 34056980..b2050e4f 100644 --- a/e2e/virtual-servers.spec.ts +++ b/e2e/virtual-servers.spec.ts @@ -439,7 +439,7 @@ test.describe("Virtual Servers page", () => { await page.getByRole("checkbox", { name: "Select github-mcp" }).check(); await page.getByRole("button", { name: "Submit" }).click(); - await expect(page.getByRole("alert")).toHaveText("Unable to load tools"); + await expect(page.getByRole("alert")).toHaveText("github-mcp: Unable to load tools"); await expect(page).toHaveURL(/\/app\/gateways\/create-server$/); }); diff --git a/src/components/gateways/SourceSelection.test.tsx b/src/components/gateways/SourceSelection.test.tsx index 970fc1d3..f1daf2ce 100644 --- a/src/components/gateways/SourceSelection.test.tsx +++ b/src/components/gateways/SourceSelection.test.tsx @@ -366,4 +366,199 @@ describe("SourceSelection", () => { expect(grpcAction).not.toHaveBeenCalled(); expect(screen.getAllByRole("button", { name: "Connect" })).toHaveLength(1); }); + it("keeps every source selectable and explains its state", async () => { + const user = userEvent.setup(); + server.use( + http.get("*/v1/mcp-servers", () => + HttpResponse.json({ + gateways: [ + { + id: "s-off", + name: "offline-src", + enabled: true, + reachable: false, + lastSeen: "2026-01-01T00:00:00Z", + visibility: "public", + tool_count: 4, + }, + { + id: "s-draft", + name: "draft-src", + enabled: false, + reachable: false, + visibility: "public", + tool_count: 0, + }, + ], + }), + ), + http.get("*/oauth/status", () => HttpResponse.json({})), + ); + + renderWithProviders( + , + ); + + await user.click( + screen.getByRole("button", { + name: "Add tools, resources, and prompts from connected sources", + }), + ); + + expect(await screen.findByText("offline-src")).toBeInTheDocument(); + expect(screen.getByText("Offline")).toBeInTheDocument(); + expect(screen.getByText("Inactive")).toBeInTheDocument(); + + // An unavailable source stays selectable: its components remain in the catalog. + for (const name of ["Select offline-src", "Select draft-src"]) { + expect(screen.getByRole("checkbox", { name })).toBeEnabled(); + } + }); + + it("warns at submit only about selected sources with nothing to add", async () => { + const user = userEvent.setup(); + server.use( + http.get("*/v1/mcp-servers", () => + HttpResponse.json({ + gateways: [ + { + id: "s-full", + name: "full-src", + enabled: true, + reachable: true, + visibility: "public", + tool_count: 3, + }, + { + id: "s-empty", + name: "empty-src", + enabled: true, + reachable: true, + visibility: "public", + tool_count: 0, + resource_count: 0, + prompt_count: 0, + }, + ], + }), + ), + http.get("*/oauth/status", () => HttpResponse.json({})), + ); + + renderWithProviders( + , + ); + + await user.click( + screen.getByRole("button", { + name: "Add tools, resources, and prompts from connected sources", + }), + ); + await screen.findByText("full-src"); + + await user.click(screen.getByRole("checkbox", { name: "Select full-src" })); + expect(screen.queryByText(/no components to add yet/)).not.toBeInTheDocument(); + + await user.click(screen.getByRole("checkbox", { name: "Select empty-src" })); + expect(screen.getByText(/empty-src has no components to add yet/)).toBeInTheDocument(); + + // Non-blocking: submitting stays available. + expect(screen.getByRole("button", { name: "Submit" })).toBeEnabled(); + }); + + it("reports the selected source names to the caller", async () => { + const user = userEvent.setup(); + const onSelectSources = vi.fn(); + server.use( + http.get("*/v1/mcp-servers", () => + HttpResponse.json({ + gateways: [ + { + id: "s-1", + name: "alpha", + enabled: true, + reachable: true, + visibility: "public", + tool_count: 1, + }, + ], + }), + ), + http.get("*/oauth/status", () => HttpResponse.json({})), + ); + + renderWithProviders( + , + ); + + await user.click( + screen.getByRole("button", { + name: "Add tools, resources, and prompts from connected sources", + }), + ); + await screen.findByText("alpha"); + await user.click(screen.getByRole("checkbox", { name: "Select alpha" })); + + expect(onSelectSources).toHaveBeenCalledWith(["s-1"], { "s-1": "alpha" }); + }); + + it("keeps the name of a selected source that has dropped out of the list", async () => { + const user = userEvent.setup(); + const onSelectSources = vi.fn(); + server.use( + http.get("*/v1/mcp-servers", () => + HttpResponse.json({ + gateways: [ + { id: "s-1", name: "alpha", enabled: true, reachable: true, tool_count: 1 }, + { id: "s-2", name: "beta", enabled: true, reachable: true, tool_count: 1 }, + ], + }), + ), + http.get("*/oauth/status", () => HttpResponse.json({})), + ); + + const { rerender } = renderWithProviders( + , + ); + + await user.click( + screen.getByRole("button", { + name: "Add tools, resources, and prompts from connected sources", + }), + ); + await screen.findByText("beta"); + await user.click(screen.getByRole("checkbox", { name: "Select beta" })); + + // beta leaves the available list while it is still selected. + rerender( + , + ); + expect(screen.queryByText("beta")).not.toBeInTheDocument(); + + await user.click(screen.getByRole("checkbox", { name: "Select alpha" })); + + expect(onSelectSources).toHaveBeenLastCalledWith(["s-2", "s-1"], { + "s-1": "alpha", + "s-2": "beta", + }); + }); }); diff --git a/src/components/gateways/SourceSelection.tsx b/src/components/gateways/SourceSelection.tsx index 97d6df7e..1a57ad9c 100644 --- a/src/components/gateways/SourceSelection.tsx +++ b/src/components/gateways/SourceSelection.tsx @@ -1,4 +1,4 @@ -import { useMemo, useState } from "react"; +import { useMemo, useRef, useState } from "react"; import { useIntl } from "react-intl"; import { ArrowLeft, @@ -61,6 +61,10 @@ function getPromptCount(server: ListedMCPServer) { return server.promptCount ?? server.prompt_count ?? 0; } +function getComponentTotal(server: ListedMCPServer) { + return getToolCount(server) + getResourceCount(server) + getPromptCount(server); +} + function getVisibilityConfig(visibility: ListedMCPServer["visibility"]) { switch (visibility) { case "private": @@ -80,7 +84,7 @@ export function SourceSelection({ }: { actionCards: ActionCard[]; associatedMCPServerIds?: string[]; - onSelectSources?: (selectedIds: string[]) => void; + onSelectSources?: (selectedIds: string[], namesById: Record) => void; createServerActions?: { onBack: () => void; onSkip: () => void; @@ -98,6 +102,7 @@ export function SourceSelection({ const [isComponentsPanelOpen, setIsComponentsPanelOpen] = useState(false); const [hasRequestedMCPServers, setHasRequestedMCPServers] = useState(false); const [selectedMCPServerIds, setSelectedMCPServerIds] = useState>(new Set()); + const selectedNamesRef = useRef>({}); const { data: mcpServersData, error: mcpServersError, @@ -127,6 +132,16 @@ export function SourceSelection({ const hasSelectedMCPServers = selectedMCPServerIds.size > 0; const panelId = "connected-sources-panel"; + // Selecting an offline source still works: its components stay in the catalog. + // Only a source with nothing to contribute leaves the virtual server empty. + const emptySelectedSources = useMemo( + () => + availableMCPServers.filter( + (server) => selectedMCPServerIds.has(server.id) && getComponentTotal(server) === 0, + ), + [availableMCPServers, selectedMCPServerIds], + ); + const handleToggleComponentsPanel = () => { setIsComponentsPanelOpen((open) => !open); setHasRequestedMCPServers(true); @@ -137,7 +152,16 @@ export function SourceSelection({ if (checked) next.add(serverId); else next.delete(serverId); setSelectedMCPServerIds(next); - onSelectSources?.(Array.from(next)); + + // Kept from when each source was picked, so a refetch that drops one does not lose its name. + const names = selectedNamesRef.current; + if (checked) { + const selected = availableMCPServers.find((server) => server.id === serverId); + if (selected) names[serverId] = selected.name; + } else { + delete names[serverId]; + } + onSelectSources?.(Array.from(next), { ...names }); }; return ( @@ -419,6 +443,18 @@ export function SourceSelection({ )} + {emptySelectedSources.length > 0 && ( + + {intl.formatMessage( + { id: "gateways.source.emptySelectionWarning" }, + { + count: emptySelectedSources.length, + names: emptySelectedSources.map((server) => server.name).join(", "), + }, + )} + + )} + { expect(isAuthorizationAvailability("inactive")).toBe(false); }); }); + +describe("getAvailabilityPresentation", () => { + it("keeps the empty state off the detail text, which assumes components exist", () => { + for (const availability of AVAILABILITIES) { + const presentation = getAvailabilityPresentation(availability); + expect(presentation.emptyId).not.toBe(presentation.detailId); + } + }); + + it("resolves every message id it hands out", () => { + const keys = Object.keys(enMessages); + for (const availability of AVAILABILITIES) { + const { labelId, shortLabelId, detailId, emptyId } = + getAvailabilityPresentation(availability); + for (const id of [labelId, shortLabelId, detailId, emptyId]) { + expect(keys, `${availability} -> ${id}`).toContain(id); + } + } + }); +}); diff --git a/src/lib/serverStatus.ts b/src/lib/serverStatus.ts index 9a5f2477..8932f799 100644 --- a/src/lib/serverStatus.ts +++ b/src/lib/serverStatus.ts @@ -41,6 +41,8 @@ interface AvailabilityPresentation { labelId: string; shortLabelId: string; detailId: string; + /** Shown where the server contributes nothing, which `detailId` does not fit. */ + emptyId: string; } const PRESENTATION: Record = { @@ -50,6 +52,7 @@ const PRESENTATION: Record = { labelId: "mcpServer.status.active", shortLabelId: "mcpServer.status.active", detailId: "mcpServer.status.detail.active", + emptyId: "gateways.details.noComponentsFound", }, authorization_required: { Icon: STATUS_ICON.warning, @@ -57,6 +60,7 @@ const PRESENTATION: Record = { labelId: "mcpServer.status.authRequired", shortLabelId: "mcpServer.status.auth.short", detailId: "mcpServer.status.detail.authRequired", + emptyId: "mcpServer.status.empty.authRequired", }, authorization_expired: { Icon: STATUS_ICON.error, @@ -64,6 +68,7 @@ const PRESENTATION: Record = { labelId: "mcpServer.status.authExpired", shortLabelId: "mcpServer.status.auth.short", detailId: "mcpServer.status.detail.authExpired", + emptyId: "mcpServer.status.empty.authExpired", }, authorization_expiring: { Icon: STATUS_ICON.warning, @@ -71,6 +76,8 @@ const PRESENTATION: Record = { labelId: "mcpServer.status.authExpiring", shortLabelId: "mcpServer.status.auth.short", detailId: "mcpServer.status.detail.authExpiring", + // Authorization still holds, so an empty list is not an authorization problem. + emptyId: "gateways.details.noComponentsFound", }, authorization_checking: { Icon: CircleDashed, @@ -78,6 +85,7 @@ const PRESENTATION: Record = { labelId: "mcpServer.status.authChecking", shortLabelId: "mcpServer.status.authChecking", detailId: "mcpServer.status.detail.authChecking", + emptyId: "mcpServer.status.empty.authChecking", }, authorization_unavailable: { Icon: STATUS_ICON.warning, @@ -85,6 +93,7 @@ const PRESENTATION: Record = { labelId: "mcpServer.status.authUnavailable", shortLabelId: "mcpServer.status.unavailable", detailId: "mcpServer.status.detail.authUnavailable", + emptyId: "mcpServer.status.empty.authUnavailable", }, unreachable: { Icon: CircleSlash, @@ -92,6 +101,7 @@ const PRESENTATION: Record = { labelId: "mcpServer.status.offline", shortLabelId: "mcpServer.status.offline", detailId: "mcpServer.status.detail.unreachable", + emptyId: "mcpServer.status.empty.unreachable", }, checking: { Icon: CircleDashed, @@ -99,6 +109,7 @@ const PRESENTATION: Record = { labelId: "mcpServer.status.checking", shortLabelId: "mcpServer.status.checking", detailId: "mcpServer.status.detail.checking", + emptyId: "mcpServer.status.empty.checking", }, inactive: { Icon: CircleDashed, @@ -106,6 +117,7 @@ const PRESENTATION: Record = { labelId: "mcpServer.status.inactive", shortLabelId: "mcpServer.status.inactive", detailId: "mcpServer.status.detail.inactive", + emptyId: "mcpServer.status.empty.inactive", }, }; diff --git a/src/pages/CreateServer.test.tsx b/src/pages/CreateServer.test.tsx index 1c015ca2..35e68d45 100644 --- a/src/pages/CreateServer.test.tsx +++ b/src/pages/CreateServer.test.tsx @@ -491,6 +491,47 @@ describe("CreateServer", () => { expect(mockNavigate).not.toHaveBeenCalledWith("/app/gateways"); }); + it("names every source whose components could not be loaded", async () => { + const user = userEvent.setup(); + server.use( + http.get("*/v1/mcp-servers", () => + HttpResponse.json({ + gateways: [ + { id: "s-1", name: "alpha", enabled: true, reachable: true, tool_count: 1 }, + { id: "s-2", name: "beta", enabled: true, reachable: true, tool_count: 1 }, + ], + }), + ), + http.get("*/oauth/status", () => HttpResponse.json({})), + http.get("*/tools", ({ request }) => { + const gatewayId = new URL(request.url).searchParams.get("gateway_id"); + return HttpResponse.json({ message: `${gatewayId} is down` }, { status: 500 }); + }), + http.get("*/resources", () => HttpResponse.json({ resources: [] })), + http.get("*/prompts", () => HttpResponse.json({ prompts: [] })), + ); + + renderWithProviders(); + + await user.type(screen.getByLabelText(/Name/), "Research server"); + await user.click(screen.getByRole("button", { name: /Continue/ })); + await screen.findByRole("heading", { name: "Connect a source" }); + await user.click( + screen.getByRole("button", { + name: "Add tools, resources, and prompts from connected sources", + }), + ); + await screen.findByText("alpha"); + await user.click(screen.getByRole("checkbox", { name: "Select alpha" })); + await user.click(screen.getByRole("checkbox", { name: "Select beta" })); + await user.click(screen.getByRole("button", { name: "Submit" })); + + const alert = await screen.findByRole("alert"); + expect(alert).toHaveTextContent("alpha: s-1 is down"); + expect(alert).toHaveTextContent("beta: s-2 is down"); + expect(mockCreateVirtualServer).not.toHaveBeenCalled(); + }); + it("opens the MCP server connection form from the post-create source selection", async () => { const user = userEvent.setup(); renderWithProviders(); @@ -621,7 +662,7 @@ describe("CreateServer", () => { expect(screen.getByText("Offline Server")).toBeInTheDocument(); expect(screen.getByText("Draft Server")).toBeInTheDocument(); - // Status labels exercise every getServerStatus / getStatusConfig branch. + // Status labels exercise every getServerAvailability branch. expect(screen.getByText("Active")).toBeInTheDocument(); expect(screen.getByText("Offline")).toBeInTheDocument(); expect(screen.getByText("Connecting")).toBeInTheDocument(); @@ -954,7 +995,7 @@ describe("CreateServer", () => { }); }); - it("renders componentError alert when tools fetch fails inside accordion", async () => { + it("names the list that failed and offers a retry inside the accordion", async () => { routerMock.path = "/app/gateways/create-server?editServerId=gateway-1"; server.use( http.get("*/v1/virtual-servers/gateway-1", () => { @@ -997,8 +1038,11 @@ describe("CreateServer", () => { }); await waitFor(() => { - expect(screen.getByText("HTTP 500")).toBeInTheDocument(); + expect(screen.getByText(/Could not load Tools: HTTP 500/)).toBeInTheDocument(); }); + expect(screen.getByRole("button", { name: "Retry" })).toBeInTheDocument(); + // Resources and prompts loaded, so they must not be reported as failures. + expect(screen.queryByText(/Could not load Resources/)).not.toBeInTheDocument(); }); it("renders fallback error message when editServerError has no message", async () => { diff --git a/src/pages/CreateServer.tsx b/src/pages/CreateServer.tsx index bb323ca5..a7ba451b 100644 --- a/src/pages/CreateServer.tsx +++ b/src/pages/CreateServer.tsx @@ -15,6 +15,7 @@ import { AccordionTrigger, } from "@/components/ui/accordion"; import { Checkbox } from "@/components/ui/checkbox"; +import { InlineNotification } from "@/components/ui/inline-notification"; import { Loading } from "@/components/ui/loading"; import { TruncatedText } from "@/components/ui/truncated-text"; import { TruncatedMiddleText } from "@/components/ui/truncated-middle-text"; @@ -25,6 +26,7 @@ import { useOAuthStatuses } from "@/hooks/useOAuthStatuses"; import { useQuery } from "@/hooks/useQuery"; import { useRouter } from "@/router"; import { + getAvailabilityPresentation, getServerAvailability, isAuthorizationAvailability, isOAuthServer, @@ -196,6 +198,13 @@ function readEditServerIdFromPath(path: string): string | null { } function getCreateServerError(error: unknown, fallbackMessage: string): string { + if (error instanceof SourceComponentsError) { + return error.failures + .map( + ({ serverName, cause }) => `${serverName}: ${getCreateServerError(cause, fallbackMessage)}`, + ) + .join("; "); + } if (error instanceof ApiError) { const body = error.body as { message?: string; detail?: unknown } | null; if (body?.message) return body.message; @@ -216,10 +225,25 @@ function getCreateServerError(error: unknown, fallbackMessage: string): string { return fallbackMessage; } +interface SourceComponentsFailure { + serverName: string; + cause: unknown; +} + +/** Names the failing sources, so a bad source does not surface as an unattributed error. */ +class SourceComponentsError extends Error { + constructor(readonly failures: SourceComponentsFailure[]) { + super(failures.map(({ serverName }) => serverName).join(", ")); + this.name = "SourceComponentsError"; + } +} + async function getComponentsForSelectedMCPServers( mcpServerIds: string[], + serverNamesById: Record = {}, ): Promise { - const componentGroups = await Promise.all( + // Settled rather than all: every failing source has to be named, not just the first to reject. + const results = await Promise.allSettled( mcpServerIds.map(async (serverId) => { const [tools, resources, prompts] = await Promise.all([ getAllGatewayComponents("tools", "tools", serverId), @@ -235,6 +259,17 @@ async function getComponentsForSelectedMCPServers( }), ); + const failures = results.flatMap((result, index) => { + if (result.status !== "rejected") return []; + const serverId = mcpServerIds[index]; + return [{ serverName: serverNamesById[serverId] ?? serverId, cause: result.reason }]; + }); + if (failures.length > 0) throw new SourceComponentsError(failures); + + const componentGroups = results.flatMap((result) => + result.status === "fulfilled" ? [result.value] : [], + ); + return { tools: uniqueStrings(componentGroups.flatMap((group) => group.tools)), resources: uniqueStrings(componentGroups.flatMap((group) => group.resources)), @@ -361,6 +396,7 @@ const MCPServerAccordionItem = memo(function MCPServerAccordionItem({ data: toolsData, error: toolsError, isLoading: toolsLoading, + refetch: refetchTools, } = useQuery( `/tools?limit=1000&include_inactive=true&gateway_id=${encodeURIComponent(server.id)}`, { enabled: isOpen }, @@ -369,6 +405,7 @@ const MCPServerAccordionItem = memo(function MCPServerAccordionItem({ data: resourcesData, error: resourcesError, isLoading: resourcesLoading, + refetch: refetchResources, } = useQuery( `/resources?limit=1000&include_inactive=true&gateway_id=${encodeURIComponent(server.id)}`, { enabled: isOpen }, @@ -377,6 +414,7 @@ const MCPServerAccordionItem = memo(function MCPServerAccordionItem({ data: promptsData, error: promptsError, isLoading: promptsLoading, + refetch: refetchPrompts, } = useQuery( `/prompts?limit=1000&include_inactive=true&gateway_id=${encodeURIComponent(server.id)}`, { enabled: isOpen }, @@ -409,8 +447,12 @@ const MCPServerAccordionItem = memo(function MCPServerAccordionItem({ [promptsData], ); const isLoadingComponents = toolsLoading || resourcesLoading || promptsLoading; - const componentError = toolsError ?? resourcesError ?? promptsError; const hasComponents = tools.length + resources.length + prompts.length > 0; + const failedLists = [ + { kind: "tools", error: toolsError, retry: refetchTools }, + { kind: "resources", error: resourcesError, retry: refetchResources }, + { kind: "prompts", error: promptsError, retry: refetchPrompts }, + ].filter((list) => list.error); return ( @@ -498,22 +540,34 @@ const MCPServerAccordionItem = memo(function MCPServerAccordionItem({ )} - {!isLoadingComponents && componentError && ( - - {componentError.message} - - )} + {!isLoadingComponents && + failedLists.map((list) => ( + { + list.retry().catch(() => undefined); + }, + }} + /> + ))} - {!isLoadingComponents && !componentError && !hasComponents && ( + {!isLoadingComponents && failedLists.length === 0 && !hasComponents && ( - {intl.formatMessage({ id: "gateways.details.noComponentsFound" })} + {intl.formatMessage({ id: getAvailabilityPresentation(availability).emptyId })} )} - {!isLoadingComponents && !componentError && hasComponents && ( + {!isLoadingComponents && hasComponents && ( ("details"); const [serverDetails, setServerDetails] = useState(null); const [selectedSourceIds, setSelectedSourceIds] = useState([]); + const [selectedSourceNames, setSelectedSourceNames] = useState>({}); const [selectedComponents, setSelectedComponents] = useState({ tools: [], resources: [], @@ -744,7 +799,7 @@ export function CreateServer() { try { const selectedSourceComponents = selectedSourceIds.length > 0 - ? await getComponentsForSelectedMCPServers(selectedSourceIds) + ? await getComponentsForSelectedMCPServers(selectedSourceIds, selectedSourceNames) : null; const detailsWithSources = { ...serverDetails, @@ -813,7 +868,10 @@ export function CreateServer() { { + setSelectedSourceIds(ids); + setSelectedSourceNames(namesById); + }} createServerActions={{ onBack: () => setStep("details"), onSkip: handleSkipForNow, diff --git a/src/types/server.ts b/src/types/server.ts index 8835282f..30c556e0 100644 --- a/src/types/server.ts +++ b/src/types/server.ts @@ -68,7 +68,18 @@ export interface ServersResponse { nextCursor?: string | null; } -export type ServerStatus = "draft" | "active" | "offline" | "warning"; +/** `GET /oauth/status` payload. Hand-built server-side, so the keys stay snake_case. */ +export interface GatewayOAuthStatus { + oauth_enabled: boolean; + grant_type?: string; + authorization_url?: string; + message?: string; + user_token_status?: { + status: string; + authorized: boolean; + expires_at?: string | null; + }; +} export interface VirtualServerTag { id?: string;
+ {intl.formatMessage( + { id: "gateways.source.emptySelectionWarning" }, + { + count: emptySelectedSources.length, + names: emptySelectedSources.map((server) => server.name).join(", "), + }, + )} +
- {componentError.message} -
- {intl.formatMessage({ id: "gateways.details.noComponentsFound" })} + {intl.formatMessage({ id: getAvailabilityPresentation(availability).emptyId })}