diff --git a/e2e/server-catalog.spec.ts b/e2e/server-catalog.spec.ts index d7a32de..64b15f1 100644 --- a/e2e/server-catalog.spec.ts +++ b/e2e/server-catalog.spec.ts @@ -1,7 +1,11 @@ -import { test, expect } from "./fixtures/api-mock"; +import { test, expect, MOCK_CSRF_TOKEN } from "./fixtures/api-mock"; import { APP } from "./utils/paths"; import type { CatalogServer } from "../src/generated/types"; +const CATALOG_ROUTE = (url: URL) => /^(?:\/api)?\/v1\/catalog$/.test(url.pathname); +const REGISTER_ROUTE = (url: URL) => + /^(?:\/api)?\/v1\/catalog\/open-notes\/register$/.test(url.pathname); + const OPEN_CONNECTED: CatalogServer = { id: "open-connected", name: "Globalping", @@ -27,6 +31,20 @@ const OPEN_AVAILABLE: CatalogServer = { is_registered: false, }; +const OPEN_SERVER = { + id: "open-notes", + name: "Public Notes", + category: "Productivity", + url: "https://notes.example/mcp", + auth_type: "Open", + provider: "Example", + description: "Search public notes and documents", + tags: ["search", "documents"], + transport: "STREAMABLEHTTP", + is_available: true, + is_registered: false, +}; + const API_KEY_SERVER: CatalogServer = { id: "api-key", name: "Secret Service", @@ -137,8 +155,9 @@ test.describe("Server catalog page", () => { await page.goto(APP.SERVER_CATALOG); await page.waitForLoadState("networkidle"); - const viewButton = page.getByRole("button", { name: "View Globalping" }); - await viewButton.click(); + const actionsButton = page.getByRole("button", { name: "Actions for Globalping" }); + await actionsButton.click(); + await page.getByRole("menuitem", { name: "View details" }).click(); const dialog = page.getByRole("dialog"); await expect(dialog.getByRole("heading", { name: "Globalping" })).toBeVisible(); @@ -151,7 +170,7 @@ test.describe("Server catalog page", () => { await page.keyboard.press("Escape"); await expect(dialog).toHaveCount(0); - await expect(viewButton).toBeFocused(); + await expect(actionsButton).toBeFocused(); await page.getByRole("button", { name: "View Public Notes" }).click(); await expect(page.getByRole("dialog").getByText("Not connected")).toBeVisible(); @@ -203,4 +222,52 @@ test.describe("Server catalog page", () => { await expect(page.getByRole("heading", { name: "Globalping" })).toBeVisible(); }); + + test("adds an open server and refreshes its card to Connected", async ({ page }) => { + let registered = false; + let registerCalls = 0; + + await page.route(CATALOG_ROUTE, async (route) => { + await route.fulfill({ + status: 200, + contentType: "application/json", + body: JSON.stringify({ + servers: [{ ...OPEN_SERVER, is_registered: registered }], + total: 1, + categories: ["Productivity"], + auth_types: ["Open"], + providers: ["Example"], + all_tags: ["search", "documents"], + }), + }); + }); + + await page.route(REGISTER_ROUTE, async (route) => { + expect(route.request().method()).toBe("POST"); + expect(route.request().headers()["x-csrf-token"]).toBe(MOCK_CSRF_TOKEN); + registerCalls += 1; + registered = true; + await route.fulfill({ + status: 200, + contentType: "application/json", + body: JSON.stringify({ + success: true, + server_id: "gateway-public-notes", + message: "Server registered successfully", + }), + }); + }); + + await page.goto(APP.SERVER_CATALOG); + await expect(page.getByRole("heading", { name: "Public Notes" })).toBeVisible(); + + await page.getByRole("button", { name: "Add Public Notes" }).click(); + + await expect.poll(() => registerCalls).toBe(1); + const catalog = page.getByRole("list", { name: "Catalog servers" }); + await expect(catalog.getByText("Connected", { exact: true })).toBeVisible(); + await expect(page.getByRole("button", { name: "Add Public Notes" })).toHaveCount(0); + await expect(page.getByRole("button", { name: "View Public Notes" })).toHaveCount(0); + await expect(page.getByRole("button", { name: "Actions for Public Notes" })).toBeVisible(); + }); }); diff --git a/src/api/catalog.test.ts b/src/api/catalog.test.ts new file mode 100644 index 0000000..51c3d7b --- /dev/null +++ b/src/api/catalog.test.ts @@ -0,0 +1,30 @@ +import { describe, expect, it } from "vitest"; +import { http, HttpResponse } from "msw"; + +import { server } from "@/test/mocks/server"; +import { registerCatalogServer } from "./catalog"; + +describe("registerCatalogServer", () => { + it("POSTs the URL-encoded catalog id through the API proxy", async () => { + let requestPath = ""; + server.use( + http.post("*/api/v1/catalog/:catalogId/register", ({ request }) => { + requestPath = new URL(request.url).pathname; + return HttpResponse.json({ + success: true, + server_id: "gateway-1", + message: "Registered", + }); + }), + ); + + const result = await registerCatalogServer("server/id with space"); + + expect(requestPath).toBe("/api/v1/catalog/server%2Fid%20with%20space/register"); + expect(result).toEqual({ + success: true, + server_id: "gateway-1", + message: "Registered", + }); + }); +}); diff --git a/src/api/catalog.ts b/src/api/catalog.ts new file mode 100644 index 0000000..cdf08a0 --- /dev/null +++ b/src/api/catalog.ts @@ -0,0 +1,11 @@ +import { api } from "./client"; +import type { CatalogServerRegisterResponse } from "@/generated/types"; + +/** Register an open catalog entry through the authenticated BFF proxy. */ +export async function registerCatalogServer( + catalogId: string, +): Promise { + return api.post( + `/v1/catalog/${encodeURIComponent(catalogId)}/register`, + ); +} diff --git a/src/components/server-catalog/CatalogResults.test.tsx b/src/components/server-catalog/CatalogResults.test.tsx new file mode 100644 index 0000000..7a7c5a9 --- /dev/null +++ b/src/components/server-catalog/CatalogResults.test.tsx @@ -0,0 +1,67 @@ +import { describe, expect, it, vi } from "vitest"; +import { screen, waitFor } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; + +import type { CatalogServer } from "@/generated/types"; +import { renderWithProviders } from "@/test/test-utils"; +import { CatalogResults } from "./CatalogResults"; + +const availableServer: CatalogServer = { + id: "public-notes", + name: "Public Notes", + category: "Productivity", + url: "https://notes.example/mcp", + auth_type: "Open", + provider: "Example", + description: "Search public notes and documents", + tags: ["search", "documents"], + is_registered: false, +}; + +function catalogResults(server: CatalogServer, addingServerIds: ReadonlySet = new Set()) { + return ( + + ); +} + +describe("CatalogResults", () => { + it("gives add states a server-specific accessible name", () => { + const { rerender } = renderWithProviders(catalogResults(availableServer)); + + expect(screen.getByRole("button", { name: "Add Public Notes" })).toBeInTheDocument(); + + rerender(catalogResults(availableServer, new Set([availableServer.id]))); + + expect(screen.getByRole("button", { name: "Adding Public Notes…" })).toBeDisabled(); + }); + + it("moves focus from Add to Actions after registration", async () => { + const user = userEvent.setup(); + const { rerender } = renderWithProviders(catalogResults(availableServer)); + + const addButton = screen.getByRole("button", { name: "Add Public Notes" }); + await user.click(addButton); + expect(addButton).toHaveFocus(); + + rerender(catalogResults({ ...availableServer, is_registered: true })); + + await waitFor(() => + expect(screen.getByRole("button", { name: "Actions for Public Notes" })).toHaveFocus(), + ); + }); + + it("aligns the Actions menu to the card's trailing edge", async () => { + const user = userEvent.setup(); + renderWithProviders(catalogResults({ ...availableServer, is_registered: true })); + + await user.click(screen.getByRole("button", { name: "Actions for Public Notes" })); + + expect(await screen.findByRole("menu")).toHaveAttribute("data-align", "end"); + }); +}); diff --git a/src/components/server-catalog/CatalogResults.tsx b/src/components/server-catalog/CatalogResults.tsx index 1bb9b95..0775a47 100644 --- a/src/components/server-catalog/CatalogResults.tsx +++ b/src/components/server-catalog/CatalogResults.tsx @@ -1,9 +1,9 @@ -import { useId, useState } from "react"; +import { useEffect, useId, useRef, useState } from "react"; import type { ReactNode } from "react"; +import { CircleCheck, EllipsisVertical, FileText, Plus } from "lucide-react"; import { useIntl } from "react-intl"; import { EmptyStatePlaceholder } from "@/components/dashboard/EmptyStatePlaceholder"; -import { StatusDot } from "@/components/dashboard/StatusDot"; import { ServerIcon } from "@/components/servers/ServerIcon"; import { Button } from "@/components/ui/button"; import { Card, CardContent } from "@/components/ui/card"; @@ -15,14 +15,20 @@ import { DialogHeader, DialogTitle, } from "@/components/ui/dialog"; +import { + DropdownMenu, + DropdownMenuContent, + DropdownMenuItem, + DropdownMenuTrigger, +} from "@/components/ui/dropdown-menu"; import type { CatalogServer } from "@/generated/types"; import { useDebouncedValue } from "@/hooks/useDebouncedValue"; -function getSafeCatalogLogoUrl(logoUrl: string | null | undefined): string | null { - if (!logoUrl) return null; +function getSafeExternalUrl(value: string | null | undefined): string | null { + if (!value) return null; try { - const parsed = new URL(logoUrl); + const parsed = new URL(value); return parsed.protocol === "https:" && !parsed.username && !parsed.password ? parsed.href : null; @@ -33,7 +39,7 @@ function getSafeCatalogLogoUrl(logoUrl: string | null | undefined): string | nul function CatalogLogo({ server }: { server: CatalogServer }) { const [failedLogoUrl, setFailedLogoUrl] = useState(null); - const logoUrl = getSafeCatalogLogoUrl(server.logo_url); + const logoUrl = getSafeExternalUrl(server.logo_url); if (!logoUrl || failedLogoUrl === logoUrl) { return ( @@ -64,12 +70,32 @@ function CatalogLogo({ server }: { server: CatalogServer }) { function CatalogCard({ server, onView, + onAdd, + isAdding, }: { server: CatalogServer; - onView: (trigger: HTMLButtonElement) => void; + onView: (trigger: HTMLElement) => void; + onAdd: () => void; + isAdding: boolean; }) { const intl = useIntl(); const headingId = useId(); + const addTriggerRef = useRef(null); + const actionsTriggerRef = useRef(null); + const pendingDetailsTriggerRef = useRef(null); + const shouldTransferAddFocusRef = useRef(false); + + useEffect(() => { + if (server.is_registered && shouldTransferAddFocusRef.current) { + shouldTransferAddFocusRef.current = false; + actionsTriggerRef.current?.focus(); + return; + } + + if (!server.is_registered && !isAdding) { + shouldTransferAddFocusRef.current = false; + } + }, [isAdding, server.is_registered]); return (
  • @@ -78,11 +104,6 @@ function CatalogCard({
    - {server.is_registered && ( - - {intl.formatMessage({ id: "mcpServer.catalog.connected" })} - - )}

    @@ -92,19 +113,91 @@ function CatalogCard({ {server.description}

    -
    - +
    + {server.is_registered ? ( + <> + + + + + + + { + const trigger = pendingDetailsTriggerRef.current; + if (!trigger) return; + event.preventDefault(); + pendingDetailsTriggerRef.current = null; + window.setTimeout(() => onView(trigger), 0); + }} + > + { + if (!actionsTriggerRef.current) return; + pendingDetailsTriggerRef.current = actionsTriggerRef.current; + }} + > + {intl.formatMessage({ id: "mcpServer.catalog.viewDetails" })} + + + + + ) : ( + + )} + + {!server.is_registered && ( + + )}
    @@ -190,10 +283,14 @@ export function CatalogResults({ servers, emptyStateMessageId, onView, + onAdd, + addingServerIds, }: { servers: CatalogServer[]; emptyStateMessageId: string; - onView: (server: CatalogServer, trigger: HTMLButtonElement) => void; + onView: (server: CatalogServer, trigger: HTMLElement) => void; + onAdd: (server: CatalogServer) => void; + addingServerIds: ReadonlySet; }) { const intl = useIntl(); const announcedCount = useDebouncedValue(servers.length, 300); @@ -213,6 +310,8 @@ export function CatalogResults({ key={server.id} server={server} onView={(trigger) => onView(server, trigger)} + onAdd={() => onAdd(server)} + isAdding={addingServerIds.has(server.id)} /> ))} diff --git a/src/hooks/useQuery.test.ts b/src/hooks/useQuery.test.ts index 9714ce3..6ae7d59 100644 --- a/src/hooks/useQuery.test.ts +++ b/src/hooks/useQuery.test.ts @@ -251,6 +251,66 @@ describe("useQuery", () => { await waitFor(() => expect(result.current.data?.count).toBe(2)); }); + + it("keeps cached data visible while a refetch is pending and after it fails", async () => { + let callCount = 0; + let releaseRefresh!: () => void; + const refreshGate = new Promise((resolve) => { + releaseRefresh = resolve; + }); + server.use( + http.get("/api/test-refetch-stale", async () => { + callCount += 1; + if (callCount === 1) return HttpResponse.json({ value: "cached" }); + + await refreshGate; + return HttpResponse.json({ detail: "refresh failed" }, { status: 500 }); + }), + ); + + const { result } = renderHook(() => useQuery<{ value: string }>("/api/test-refetch-stale")); + await waitFor(() => expect(result.current.data).toEqual({ value: "cached" })); + + let refetchPromise!: Promise<{ value: string }>; + act(() => { + refetchPromise = result.current.refetch(); + }); + await waitFor(() => expect(result.current.isLoading).toBe(true)); + expect(result.current.data).toEqual({ value: "cached" }); + + await act(async () => { + releaseRefresh(); + await expect(refetchPromise).rejects.toMatchObject({ status: 500 }); + }); + + expect(result.current.data).toEqual({ value: "cached" }); + expect(result.current.error).toMatchObject({ status: 500 }); + expect(result.current.isLoading).toBe(false); + }); + + it("replaces an optimistic cache update with later authoritative data", async () => { + server.use( + http.get("/api/test-refetch-authoritative", () => + HttpResponse.json({ is_registered: false }), + ), + ); + + const { result } = renderHook(() => + useQuery<{ is_registered: boolean }>("/api/test-refetch-authoritative"), + ); + await waitFor(() => expect(result.current.data).toEqual({ is_registered: false })); + + act(() => { + result.current.setData({ is_registered: true }); + }); + expect(result.current.data).toEqual({ is_registered: true }); + + await act(async () => { + await result.current.refetch(); + }); + + expect(result.current.data).toEqual({ is_registered: false }); + }); }); describe("execute with overrideBody", () => { diff --git a/src/i18n/locales/en-US/mcpServer.json b/src/i18n/locales/en-US/mcpServer.json index 7215ead..390bc53 100644 --- a/src/i18n/locales/en-US/mcpServer.json +++ b/src/i18n/locales/en-US/mcpServer.json @@ -24,8 +24,16 @@ "mcpServer.catalog.selectTags": "Select...", "mcpServer.catalog.connected": "Connected", "mcpServer.catalog.notConnected": "Not connected", - "mcpServer.catalog.view": "View", "mcpServer.catalog.viewServer": "View {name}", + "mcpServer.catalog.add": "Add", + "mcpServer.catalog.adding": "Adding…", + "mcpServer.catalog.addServer": "Add {name}", + "mcpServer.catalog.addingServer": "Adding {name}…", + "mcpServer.catalog.addError": "Unable to add this server. Try again.", + "mcpServer.catalog.addNotFound": "{name} is no longer available in the catalog.", + "mcpServer.catalog.alreadyConnected": "{name} is already connected.", + "mcpServer.catalog.actionsFor": "Actions for {name}", + "mcpServer.catalog.viewDetails": "View details", "mcpServer.catalog.viewOptions": "Catalog view", "mcpServer.catalog.transport": "Transport", "mcpServer.catalog.status": "Status", diff --git a/src/i18n/locales/es-ES/mcpServer.json b/src/i18n/locales/es-ES/mcpServer.json index 5d7d329..0fe3cb9 100644 --- a/src/i18n/locales/es-ES/mcpServer.json +++ b/src/i18n/locales/es-ES/mcpServer.json @@ -24,8 +24,16 @@ "mcpServer.catalog.selectTags": "Seleccionar...", "mcpServer.catalog.connected": "Conectado", "mcpServer.catalog.notConnected": "No conectado", - "mcpServer.catalog.view": "Ver", "mcpServer.catalog.viewServer": "Ver {name}", + "mcpServer.catalog.add": "Añadir", + "mcpServer.catalog.adding": "Añadiendo…", + "mcpServer.catalog.addServer": "Añadir {name}", + "mcpServer.catalog.addingServer": "Añadiendo {name}…", + "mcpServer.catalog.addError": "No se pudo añadir este servidor. Inténtalo de nuevo.", + "mcpServer.catalog.addNotFound": "{name} ya no está disponible en el catálogo.", + "mcpServer.catalog.alreadyConnected": "{name} ya está conectado.", + "mcpServer.catalog.actionsFor": "Acciones para {name}", + "mcpServer.catalog.viewDetails": "Ver detalles", "mcpServer.catalog.viewOptions": "Vista del catálogo", "mcpServer.catalog.transport": "Transporte", "mcpServer.catalog.status": "Estado", diff --git a/src/i18n/locales/pt-BR/mcpServer.json b/src/i18n/locales/pt-BR/mcpServer.json index 5eaa49f..2fa1f8b 100644 --- a/src/i18n/locales/pt-BR/mcpServer.json +++ b/src/i18n/locales/pt-BR/mcpServer.json @@ -24,8 +24,16 @@ "mcpServer.catalog.selectTags": "Selecionar...", "mcpServer.catalog.connected": "Conectado", "mcpServer.catalog.notConnected": "Não conectado", - "mcpServer.catalog.view": "Ver", "mcpServer.catalog.viewServer": "Ver {name}", + "mcpServer.catalog.add": "Adicionar", + "mcpServer.catalog.adding": "Adicionando…", + "mcpServer.catalog.addServer": "Adicionar {name}", + "mcpServer.catalog.addingServer": "Adicionando {name}…", + "mcpServer.catalog.addError": "Não foi possível adicionar este servidor. Tente novamente.", + "mcpServer.catalog.addNotFound": "{name} não está mais disponível no catálogo.", + "mcpServer.catalog.alreadyConnected": "{name} já está conectado.", + "mcpServer.catalog.actionsFor": "Ações para {name}", + "mcpServer.catalog.viewDetails": "Ver detalhes", "mcpServer.catalog.viewOptions": "Visualização do catálogo", "mcpServer.catalog.transport": "Transporte", "mcpServer.catalog.status": "Status", diff --git a/src/pages/ServerCatalog.test.tsx b/src/pages/ServerCatalog.test.tsx index 92e8eec..1d75211 100644 --- a/src/pages/ServerCatalog.test.tsx +++ b/src/pages/ServerCatalog.test.tsx @@ -3,6 +3,8 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import { fireEvent, render, screen, waitFor, within } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; +import { registerCatalogServer } from "@/api/catalog"; +import { ApiError } from "@/api/client"; import type { CatalogListResponse, CatalogServer } from "@/generated/types"; import { useQuery } from "@/hooks/useQuery"; import { I18nProvider } from "@/i18n"; @@ -12,8 +14,12 @@ import { ServerCatalog } from "./ServerCatalog"; vi.mock("@/hooks/useQuery", () => ({ useQuery: vi.fn(), })); +vi.mock("@/api/catalog", () => ({ + registerCatalogServer: vi.fn(), +})); const mockUseQuery = vi.mocked(useQuery); +const mockRegisterCatalogServer = vi.mocked(registerCatalogServer); const openConnected: CatalogServer = { id: "open-connected", @@ -98,19 +104,28 @@ async function selectSectionOption(user: UserEvent, section: string, option: str await user.click(within(getFilterSection(section)).getByRole("checkbox", { name: option })); } -function renderWithRouter(ui: ReactElement, path = "/app/server-catalog") { - window.history.pushState({}, "", path); - return render( +function withProviders(ui: ReactElement) { + return ( {ui} - , + ); } +function renderWithRouter(ui: ReactElement, path = "/app/server-catalog") { + window.history.pushState({}, "", path); + return render(withProviders(ui)); +} + describe("ServerCatalog", () => { beforeEach(() => { window.history.replaceState({}, "", "/app/"); mockUseQuery.mockReturnValue(queryResult()); + mockRegisterCatalogServer.mockResolvedValue({ + success: true, + server_id: "registered-server", + message: "Registered", + }); }); it("uses the catalog GET endpoint and shared loader", () => { @@ -122,6 +137,21 @@ describe("ServerCatalog", () => { expect(screen.getByRole("status", { name: "Loading..." })).toBeInTheDocument(); }); + it("keeps cached catalog data visible during refreshes and refresh failures", () => { + let currentQueryResult = queryResult({ isLoading: true }); + mockUseQuery.mockImplementation(() => currentQueryResult); + const { rerender } = renderWithRouter(); + + expect(screen.getByRole("heading", { name: "Globalping" })).toBeInTheDocument(); + expect(screen.queryByRole("status", { name: "Loading..." })).not.toBeInTheDocument(); + + currentQueryResult = queryResult({ error: { message: "refresh failed", status: 500 } }); + rerender(withProviders()); + + expect(screen.getByRole("heading", { name: "Globalping" })).toBeInTheDocument(); + expect(screen.queryByText("Unable to load server catalog. Try again.")).not.toBeInTheDocument(); + }); + it("renders only exact Open entries and marks registered servers connected", () => { renderWithRouter(); @@ -133,7 +163,9 @@ describe("ServerCatalog", () => { expect(screen.queryByText("Secret Service")).not.toBeInTheDocument(); expect(within(catalogList).getByText("Connected")).toBeInTheDocument(); expect(screen.getByRole("status")).toHaveTextContent("2 servers shown"); - expect(screen.getByRole("button", { name: "View Globalping" })).toBeInTheDocument(); + expect(screen.getByRole("button", { name: "Actions for Globalping" })).toBeInTheDocument(); + expect(screen.getByRole("button", { name: "Add Public Notes" })).toBeInTheDocument(); + expect(screen.queryByRole("button", { name: "View Globalping" })).not.toBeInTheDocument(); expect(screen.getByRole("button", { name: "View Public Notes" })).toBeInTheDocument(); expect(screen.queryByText(/registration coming soon/i)).not.toBeInTheDocument(); }); @@ -142,10 +174,11 @@ describe("ServerCatalog", () => { const user = userEvent.setup(); renderWithRouter(); - const viewButton = screen.getByRole("button", { name: "View Globalping" }); - await user.click(viewButton); + const actionsButton = screen.getByRole("button", { name: "Actions for Globalping" }); + await user.click(actionsButton); + await user.click(screen.getByRole("menuitem", { name: "View details" })); - const dialog = screen.getByRole("dialog"); + const dialog = await screen.findByRole("dialog"); expect(within(dialog).getByRole("heading", { name: "Globalping" })).toBeInTheDocument(); expect(within(dialog).getByText("jsDelivr")).toBeInTheDocument(); expect(within(dialog).getByText("STREAMABLEHTTP")).toBeInTheDocument(); @@ -153,10 +186,159 @@ describe("ServerCatalog", () => { await user.keyboard("{Escape}"); expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); - await waitFor(() => expect(viewButton).toHaveFocus()); + await waitFor(() => expect(actionsButton).toHaveFocus()); + + await user.click(screen.getByRole("button", { name: "Add Public Notes" })); + expect(mockRegisterCatalogServer).toHaveBeenCalledWith("open-available"); + }); + + it("reports catalog registration failures", async () => { + const user = userEvent.setup(); + mockRegisterCatalogServer.mockRejectedValue(new Error("network detail must not leak")); + renderWithRouter(); + + await user.click(screen.getByRole("button", { name: "Add Public Notes" })); + + expect(await screen.findByRole("alert")).toHaveTextContent( + "Unable to add this server. Try again.", + ); + expect(screen.queryByText(/network detail/i)).not.toBeInTheDocument(); + }); + + it("registers an available server and refreshes the catalog", async () => { + const user = userEvent.setup(); + const refetch = vi.fn().mockResolvedValue(undefined); + const setData = vi.fn(); + mockUseQuery.mockReturnValue(queryResult({ refetch, setData })); + renderWithRouter(); + + await user.click(screen.getByRole("button", { name: "Add Public Notes" })); + + await waitFor(() => expect(mockRegisterCatalogServer).toHaveBeenCalledWith("open-available")); + expect(setData).toHaveBeenCalledOnce(); + const updateCatalog = setData.mock.calls[0][0] as ( + current: CatalogListResponse | undefined, + ) => CatalogListResponse | undefined; + expect( + updateCatalog(response)?.servers.find((server) => server.id === openAvailable.id), + ).toMatchObject({ is_registered: true }); + expect(refetch).toHaveBeenCalledOnce(); + }); + + it("keeps successful registration when catalog refresh fails", async () => { + const user = userEvent.setup(); + const refetch = vi.fn().mockRejectedValue(new Error("refresh failed")); + const setData = vi.fn(); + mockUseQuery.mockReturnValue(queryResult({ refetch, setData })); + renderWithRouter(); + + const card = screen.getByRole("heading", { name: "Public Notes" }).closest("article")!; + await user.click(within(card).getByRole("button", { name: "Add Public Notes" })); + + await waitFor(() => expect(refetch).toHaveBeenCalledOnce()); + expect(setData).toHaveBeenCalledOnce(); + expect(screen.queryByText("Unable to add this server. Try again.")).not.toBeInTheDocument(); + }); + + it("treats an already-registered response as connected and refreshes", async () => { + const user = userEvent.setup(); + const refetch = vi.fn().mockResolvedValue(undefined); + const setData = vi.fn(); + mockUseQuery.mockReturnValue(queryResult({ refetch, setData })); + mockRegisterCatalogServer.mockRejectedValue( + new ApiError(409, { detail: "Server already registered" }, "HTTP 409"), + ); + renderWithRouter(); + + await user.click(screen.getByRole("button", { name: "Add Public Notes" })); + + expect(await screen.findByText("Public Notes is already connected.")).toBeInTheDocument(); + expect(setData).toHaveBeenCalledOnce(); + expect(refetch).toHaveBeenCalledOnce(); + expect(screen.queryByText("Server already registered")).not.toBeInTheDocument(); + }); + + it("removes a stale catalog entry after a registration 404 and refreshes", async () => { + const user = userEvent.setup(); + const refetch = vi.fn().mockResolvedValue(undefined); + const setData = vi.fn(); + mockUseQuery.mockReturnValue(queryResult({ refetch, setData })); + mockRegisterCatalogServer.mockRejectedValue( + new ApiError(404, { detail: "Catalog server not found" }, "HTTP 404"), + ); + renderWithRouter(); + + await user.click(screen.getByRole("button", { name: "Add Public Notes" })); + + expect(await screen.findByRole("alert")).toHaveTextContent( + "Public Notes is no longer available in the catalog.", + ); + expect(setData).toHaveBeenCalledOnce(); + const updateCatalog = setData.mock.calls[0][0] as ( + current: CatalogListResponse | undefined, + ) => CatalogListResponse | undefined; + expect(updateCatalog(response)?.servers).not.toContainEqual(openAvailable); + expect(updateCatalog(response)?.total).toBe(2); + expect(refetch).toHaveBeenCalledOnce(); + expect(screen.queryByText("Catalog server not found")).not.toBeInTheDocument(); + }); + + it("tracks concurrent registrations independently", async () => { + const user = userEvent.setup(); + const secondAvailable = { + ...openAvailable, + id: "open-weather", + name: "Public Weather", + }; + let resolveNotes!: (value: Awaited>) => void; + let resolveWeather!: (value: Awaited>) => void; + mockRegisterCatalogServer.mockImplementation( + (id) => + new Promise((resolve) => { + if (id === openAvailable.id) resolveNotes = resolve; + if (id === secondAvailable.id) resolveWeather = resolve; + }), + ); + const setData = vi.fn(); + mockUseQuery.mockReturnValue( + queryResult({ + data: { ...response, servers: [openAvailable, secondAvailable], total: 2 }, + setData, + }), + ); + renderWithRouter(); + + const notesCard = screen.getByRole("heading", { name: "Public Notes" }).closest("article")!; + const weatherCard = screen.getByRole("heading", { name: "Public Weather" }).closest("article")!; + await user.click(within(notesCard).getByRole("button", { name: "Add Public Notes" })); + await user.click(within(weatherCard).getByRole("button", { name: "Add Public Weather" })); + + expect(within(notesCard).getByRole("button", { name: "Adding Public Notes…" })).toBeDisabled(); + expect( + within(weatherCard).getByRole("button", { name: "Adding Public Weather…" }), + ).toBeDisabled(); + + resolveWeather({ success: true, server_id: "weather", message: "Registered" }); + await waitFor(() => expect(setData).toHaveBeenCalledTimes(1)); + expect(within(notesCard).getByRole("button", { name: "Adding Public Notes…" })).toBeDisabled(); + + resolveNotes({ success: true, server_id: "notes", message: "Registered" }); + await waitFor(() => expect(setData).toHaveBeenCalledTimes(2)); + }); + + it("shows connected status in details opened from the action menu", async () => { + const user = userEvent.setup(); + mockUseQuery.mockReturnValue( + queryResult({ data: { ...response, servers: [{ ...openAvailable, is_registered: true }] } }), + ); + renderWithRouter(); - await user.click(screen.getByRole("button", { name: "View Public Notes" })); - expect(within(screen.getByRole("dialog")).getByText("Not connected")).toBeInTheDocument(); + const actionsButton = screen.getByRole("button", { name: "Actions for Public Notes" }); + await user.click(actionsButton); + await user.click(screen.getByRole("menuitem", { name: "View details" })); + const dialog = await screen.findByRole("dialog"); + expect(within(dialog).getByText("Connected")).toBeInTheDocument(); + await waitFor(() => expect(dialog.contains(document.activeElement)).toBe(true)); }); it("renders safe remote logos and falls back when loading fails", () => { diff --git a/src/pages/ServerCatalog.tsx b/src/pages/ServerCatalog.tsx index b982b6f..2d76746 100644 --- a/src/pages/ServerCatalog.tsx +++ b/src/pages/ServerCatalog.tsx @@ -2,6 +2,8 @@ import { useCallback, useEffect, useMemo, useRef, useState } from "react"; import type { ReactNode } from "react"; import { useIntl } from "react-intl"; +import { registerCatalogServer } from "@/api/catalog"; +import { ApiError } from "@/api/client"; import { CatalogResults, CatalogServerDetailsDialog, @@ -33,6 +35,11 @@ interface CatalogFilters { installedOnly: boolean; } +interface RegistrationNotification { + type: "success" | "error"; + message: string; +} + type QueryUpdateValue = string | string[] | boolean | null; function getQuery(path: string): string { @@ -130,6 +137,39 @@ function sortedUnique(values: Array): string[] { return [...new Set(values.filter((value): value is string => Boolean(value)))].sort(); } +function setCatalogServerRegistration( + catalog: CatalogListResponse | undefined, + serverId: string, + isRegistered: boolean, +): CatalogListResponse | undefined { + if (!catalog) return catalog; + + const serverIndex = catalog.servers.findIndex((server) => server.id === serverId); + if (serverIndex === -1 || catalog.servers[serverIndex].is_registered === isRegistered) { + return catalog; + } + + const servers = [...catalog.servers]; + servers[serverIndex] = { ...servers[serverIndex], is_registered: isRegistered }; + return { ...catalog, servers }; +} + +function removeCatalogServer( + catalog: CatalogListResponse | undefined, + serverId: string, +): CatalogListResponse | undefined { + if (!catalog) return catalog; + + const servers = catalog.servers.filter((server) => server.id !== serverId); + if (servers.length === catalog.servers.length) return catalog; + + return { + ...catalog, + servers, + total: Math.max(0, catalog.total - 1), + }; +} + function CatalogPageLayout({ children }: { children: ReactNode }) { const intl = useIntl(); @@ -146,8 +186,12 @@ function CatalogPageLayout({ children }: { children: ReactNode }) { export function ServerCatalog() { const intl = useIntl(); const [selectedServer, setSelectedServer] = useState(null); - const lastViewTriggerRef = useRef(null); - const { data, error, isLoading, refetch } = useQuery(CATALOG_PATH); + const [addingServerIds, setAddingServerIds] = useState>(() => new Set()); + const [registrationNotification, setRegistrationNotification] = + useState(null); + const addingServerIdsRef = useRef(new Set()); + const lastViewTriggerRef = useRef(null); + const { data, error, isLoading, refetch, setData } = useQuery(CATALOG_PATH); const { filters, updateQuery, applyFilters } = useCatalogFilters(); const [search, setSearch] = useState(filters.search); const debouncedSearch = useDebouncedValue(search, 300); @@ -193,18 +237,85 @@ export function ServerCatalog() { : "mcpServer.catalog.noResults"; const activeFilterCount = filters.category.length + filters.provider.length + filters.tags.length; - const handleView = useCallback((server: CatalogServer, trigger: HTMLButtonElement) => { + const handleView = useCallback((server: CatalogServer, trigger: HTMLElement) => { lastViewTriggerRef.current = trigger; setSelectedServer(server); }, []); + const refreshCatalogSilently = useCallback(async () => { + try { + await refetch(); + } catch { + // The mutation result remains reflected in the cached data. A later + // successful fetch will replace that optimistic value authoritatively. + } + }, [refetch]); + + const handleAdd = useCallback( + async (server: CatalogServer) => { + if (addingServerIdsRef.current.has(server.id)) return; + + addingServerIdsRef.current.add(server.id); + setAddingServerIds(new Set(addingServerIdsRef.current)); + setRegistrationNotification(null); + try { + const result = await registerCatalogServer(server.id); + if (!result.success) { + setRegistrationNotification({ + type: "error", + message: result.message || intl.formatMessage({ id: "mcpServer.catalog.addError" }), + }); + return; + } + + setData((current) => setCatalogServerRegistration(current, server.id, true)); + await refreshCatalogSilently(); + } catch (registrationError) { + if (registrationError instanceof ApiError && registrationError.status === 409) { + setData((current) => setCatalogServerRegistration(current, server.id, true)); + setRegistrationNotification({ + type: "success", + message: intl.formatMessage( + { id: "mcpServer.catalog.alreadyConnected" }, + { name: server.name }, + ), + }); + await refreshCatalogSilently(); + return; + } + + if (registrationError instanceof ApiError && registrationError.status === 404) { + setData((current) => removeCatalogServer(current, server.id)); + setRegistrationNotification({ + type: "error", + message: intl.formatMessage( + { id: "mcpServer.catalog.addNotFound" }, + { name: server.name }, + ), + }); + await refreshCatalogSilently(); + return; + } + + setRegistrationNotification({ + type: "error", + message: intl.formatMessage({ id: "mcpServer.catalog.addError" }), + }); + } finally { + addingServerIdsRef.current.delete(server.id); + setAddingServerIds(new Set(addingServerIdsRef.current)); + } + }, + [intl, refreshCatalogSilently, setData], + ); + const handleDetailsOpenChange = useCallback((open: boolean) => { if (open) return; setSelectedServer(null); window.setTimeout(() => lastViewTriggerRef.current?.focus(), 0); }, []); - if (isLoading) { + if (isLoading && !data) { return (
    @@ -214,7 +325,7 @@ export function ServerCatalog() { ); } - if (error?.status === 404) { + if (error?.status === 404 && !data) { return (
    @@ -224,7 +335,7 @@ export function ServerCatalog() { ); } - if (error) { + if (error && !data) { return (
    @@ -262,10 +373,22 @@ export function ServerCatalog() { onApply={applyFilters} /> + {registrationNotification && ( +
    + setRegistrationNotification(null)} + /> +
    + )} + void handleAdd(server)} + addingServerIds={addingServerIds} />