From d4ca62571e3ec76874bd33ca7ecda2c2d889e6eb Mon Sep 17 00:00:00 2001 From: Vishu Bhatnagar Date: Tue, 25 Aug 2026 15:11:48 +0100 Subject: [PATCH 1/8] feat: add catalog gateway actions Signed-off-by: Vishu Bhatnagar --- e2e/server-catalog.spec.ts | 15 +- openapi.json | 14 +- src/api/catalog.test.ts | 58 ++- src/api/catalog.ts | 38 +- src/api/client.ts | 12 +- .../server-catalog/CatalogResults.test.tsx | 4 + .../server-catalog/CatalogResults.tsx | 57 ++- src/components/servers/ConfirmDialog.tsx | 7 +- src/i18n/locales/en-US/mcpServer.json | 14 + src/i18n/locales/es-ES/mcpServer.json | 14 + src/i18n/locales/pt-BR/mcpServer.json | 14 + src/pages/ServerCatalog.test.tsx | 156 +++++++- src/pages/ServerCatalog.tsx | 345 +++++++++++++++++- 13 files changed, 715 insertions(+), 33 deletions(-) diff --git a/e2e/server-catalog.spec.ts b/e2e/server-catalog.spec.ts index 27ece6bc..421bcfa7 100644 --- a/e2e/server-catalog.spec.ts +++ b/e2e/server-catalog.spec.ts @@ -70,6 +70,7 @@ async function mockCatalog(page: import("@playwright/test").Page, servers: Catal test.describe("Server catalog page", () => { test.beforeEach(async ({ page, apiMock }) => { await apiMock.mockSession(); + await apiMock.mockPermissions(); await page.addInitScript(() => { sessionStorage.setItem("mcpgateway_token", "mock-token-12345"); @@ -224,7 +225,7 @@ test.describe("Server catalog page", () => { await expect(page.getByRole("heading", { name: "Globalping" })).toBeVisible(); }); - test("adds an open server without refetching its card", async ({ page, apiMock }) => { + test("adds an open server then refreshes its gateway id", async ({ page }) => { let registered = false; let catalogCalls = 0; let registerCalls = 0; @@ -235,7 +236,13 @@ test.describe("Server catalog page", () => { status: 200, contentType: "application/json", body: JSON.stringify({ - servers: [{ ...OPEN_SERVER, is_registered: registered }], + servers: [ + { + ...OPEN_SERVER, + is_registered: registered, + gateway_id: registered ? "gateway-public-notes" : null, + }, + ], total: 1, categories: ["Productivity"], auth_types: ["Open"], @@ -276,7 +283,9 @@ test.describe("Server catalog page", () => { 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(); - expect(catalogCalls).toBe(catalogCallsBeforeAdd); + await expect.poll(() => catalogCalls).toBe(catalogCallsBeforeAdd + 1); + await page.getByRole("button", { name: "Actions for Public Notes" }).click(); + await expect(page.getByRole("menuitem", { name: "Disconnect" })).toBeVisible(); }); test("removes a stale server and moves focus to its 404 notification", async ({ page }) => { diff --git a/openapi.json b/openapi.json index 30026ea8..7fb971f6 100644 --- a/openapi.json +++ b/openapi.json @@ -39786,6 +39786,18 @@ "description": "Whether server is already registered", "default": false }, + "gateway_id": { + "anyOf": [ + { + "type": "string" + }, + { + "type": "null" + } + ], + "title": "Gateway Id", + "description": "ID of the caller-visible gateway matched to this catalog server" + }, "is_available": { "type": "boolean", "title": "Is Available", @@ -53129,4 +53141,4 @@ } } } -} \ No newline at end of file +} diff --git a/src/api/catalog.test.ts b/src/api/catalog.test.ts index 51c3d7b5..6667f302 100644 --- a/src/api/catalog.test.ts +++ b/src/api/catalog.test.ts @@ -2,7 +2,12 @@ import { describe, expect, it } from "vitest"; import { http, HttpResponse } from "msw"; import { server } from "@/test/mocks/server"; -import { registerCatalogServer } from "./catalog"; +import { + disconnectCatalogGateway, + getGatewayImpactPreview, + registerCatalogServer, + testCatalogServer, +} from "./catalog"; describe("registerCatalogServer", () => { it("POSTs the URL-encoded catalog id through the API proxy", async () => { @@ -27,4 +32,55 @@ describe("registerCatalogServer", () => { message: "Registered", }); }); + + it("DELETEs an encoded gateway ID and preserves async lifecycle metadata", async () => { + let requestPath = ""; + server.use( + http.delete("*/api/v1/gateways/:gatewayId", ({ request }) => { + requestPath = new URL(request.url).pathname; + return HttpResponse.json( + { status: "deleting" }, + { status: 202, headers: { "Retry-After": "2" } }, + ); + }), + ); + + const result = await disconnectCatalogGateway("gateway/id"); + + expect(requestPath).toBe("/api/v1/gateways/gateway%2Fid"); + expect(result.status).toBe(202); + expect(result.headers.get("Retry-After")).toBe("2"); + }); + + it("tests catalog URL with a safe fixed GET request", async () => { + let body: unknown; + server.use( + http.post("*/api/v1/mcp-servers/test", async ({ request }) => { + body = await request.json(); + return HttpResponse.json({ statusCode: 204, latencyMs: 21 }); + }), + ); + + await expect(testCatalogServer("https://catalog.example/mcp")).resolves.toEqual({ + statusCode: 204, + latencyMs: 21, + }); + expect(body).toEqual({ method: "GET", baseUrl: "https://catalog.example/mcp", path: "" }); + }); + + it("gets only the backend-provided disconnect impact preview", async () => { + server.use( + http.get("*/api/v1/gateways/:gatewayId/impact-preview", () => + HttpResponse.json({ + gatewayId: "gateway-1", + servers: [{ id: "server-1", name: "Visible server" }], + }), + ), + ); + + await expect(getGatewayImpactPreview("gateway-1")).resolves.toEqual({ + gatewayId: "gateway-1", + servers: [{ id: "server-1", name: "Visible server" }], + }); + }); }); diff --git a/src/api/catalog.ts b/src/api/catalog.ts index cdf08a0d..0e6971c3 100644 --- a/src/api/catalog.ts +++ b/src/api/catalog.ts @@ -1,5 +1,17 @@ import { api } from "./client"; -import type { CatalogServerRegisterResponse } from "@/generated/types"; +import type { + CatalogServerRegisterResponse, + GatewayRead, + GatewayTestRequest, + GatewayTestResponse, +} from "@/generated/types"; + +export interface GatewayImpactPreview { + gatewayId: string; + servers: Array<{ id: string; name: string }>; +} + +export type CatalogGatewayDeleteResponse = GatewayRead | { status?: string; message?: string }; /** Register an open catalog entry through the authenticated BFF proxy. */ export async function registerCatalogServer( @@ -9,3 +21,27 @@ export async function registerCatalogServer( `/v1/catalog/${encodeURIComponent(catalogId)}/register`, ); } + +/** Delete gateway selected by caller-visible catalog registration state. */ +export function disconnectCatalogGateway(gatewayId: string) { + return api.deleteWithMeta( + `/v1/gateways/${encodeURIComponent(gatewayId)}`, + ); +} + +/** Test the catalog server URL using stored credentials when backend has them. */ +export function testCatalogServer(url: string): Promise { + const request: GatewayTestRequest = { + method: "GET", + baseUrl: url, + path: "", + }; + return api.post("/v1/mcp-servers/test", request); +} + +/** Preview caller-visible virtual servers affected by disconnecting a gateway. */ +export function getGatewayImpactPreview(gatewayId: string): Promise { + return api.get( + `/v1/gateways/${encodeURIComponent(gatewayId)}/impact-preview`, + ); +} diff --git a/src/api/client.ts b/src/api/client.ts index d326a28b..6e0de44e 100644 --- a/src/api/client.ts +++ b/src/api/client.ts @@ -123,6 +123,7 @@ interface RequestOptions { export interface ResponseWithMeta { data: T; status: number; + headers: Headers; } async function requestWithMeta( @@ -192,11 +193,11 @@ async function requestWithMeta( // 204 No Content if (response.status === 204) { - return { data: undefined as T, status: response.status }; + return { data: undefined as T, status: response.status, headers: response.headers }; } const data = (await response.json()) as T; - return { data, status: response.status }; + return { data, status: response.status, headers: response.headers }; } async function request(path: string, options: RequestOptions = {}): Promise { @@ -241,6 +242,13 @@ export const api = { return requestWithMeta(path, { method: "POST", body, ...opts }); }, + deleteWithMeta( + path: string, + opts?: Omit, + ): Promise> { + return requestWithMeta(path, { method: "DELETE", ...opts }); + }, + put(path: string, body?: unknown, opts?: Omit): Promise { return request(path, { method: "PUT", body, ...opts }); }, diff --git a/src/components/server-catalog/CatalogResults.test.tsx b/src/components/server-catalog/CatalogResults.test.tsx index 7a7c5a9f..a19ff3a6 100644 --- a/src/components/server-catalog/CatalogResults.test.tsx +++ b/src/components/server-catalog/CatalogResults.test.tsx @@ -26,6 +26,10 @@ function catalogResults(server: CatalogServer, addingServerIds: ReadonlySet ); } diff --git a/src/components/server-catalog/CatalogResults.tsx b/src/components/server-catalog/CatalogResults.tsx index 1dbe6956..8684cf41 100644 --- a/src/components/server-catalog/CatalogResults.tsx +++ b/src/components/server-catalog/CatalogResults.tsx @@ -1,6 +1,6 @@ import { useEffect, useId, useRef, useState } from "react"; import type { ReactNode } from "react"; -import { CircleCheck, EllipsisVertical, FileText, Plus } from "lucide-react"; +import { CircleCheck, EllipsisVertical, FileText, PlugZap, Plus, Unplug } from "lucide-react"; import { useIntl } from "react-intl"; import { EmptyStatePlaceholder } from "@/components/dashboard/EmptyStatePlaceholder"; @@ -25,6 +25,8 @@ import type { CatalogServer } from "@/generated/types"; import { useDebouncedValue } from "@/hooks/useDebouncedValue"; import { getTagLabels } from "@/utils/tags"; +const EMPTY_PENDING_IDS: ReadonlySet = new Set(); + function getSafeExternalUrl(value: string | null | undefined): string | null { if (!value) return null; @@ -72,12 +74,24 @@ function CatalogCard({ server, onView, onAdd, + onTest, + onDisconnect, isAdding, + isTesting, + isDisconnecting, + canTest, + canDisconnect, }: { server: CatalogServer; onView: (trigger: HTMLElement) => void; onAdd: () => void; + onTest: () => void; + onDisconnect: () => void; isAdding: boolean; + isTesting: boolean; + isDisconnecting: boolean; + canTest: boolean; + canDisconnect: boolean; }) { const intl = useIntl(); const headingId = useId(); @@ -154,6 +168,29 @@ function CatalogCard({ > {intl.formatMessage({ id: "mcpServer.catalog.viewDetails" })} + {canTest && ( + + + )} + {canDisconnect && server.gateway_id && ( + + + )} @@ -287,12 +324,24 @@ export function CatalogResults({ onView, onAdd, addingServerIds, + onTest, + onDisconnect, + testingServerIds = EMPTY_PENDING_IDS, + disconnectingServerIds = EMPTY_PENDING_IDS, + canTest, + canDisconnect, }: { servers: CatalogServer[]; emptyStateMessageId: string; onView: (server: CatalogServer, trigger: HTMLElement) => void; onAdd: (server: CatalogServer) => void; addingServerIds: ReadonlySet; + onTest: (server: CatalogServer) => void; + onDisconnect: (server: CatalogServer) => void; + testingServerIds?: ReadonlySet; + disconnectingServerIds?: ReadonlySet; + canTest: boolean; + canDisconnect: boolean; }) { const intl = useIntl(); const announcedCount = useDebouncedValue(servers.length, 300); @@ -313,7 +362,13 @@ export function CatalogResults({ server={server} onView={(trigger) => onView(server, trigger)} onAdd={() => onAdd(server)} + onTest={() => onTest(server)} + onDisconnect={() => onDisconnect(server)} isAdding={addingServerIds.has(server.id)} + isTesting={testingServerIds.has(server.id)} + isDisconnecting={disconnectingServerIds.has(server.id)} + canTest={canTest} + canDisconnect={canDisconnect} /> ))} diff --git a/src/components/servers/ConfirmDialog.tsx b/src/components/servers/ConfirmDialog.tsx index cc88388e..a3698bcd 100644 --- a/src/components/servers/ConfirmDialog.tsx +++ b/src/components/servers/ConfirmDialog.tsx @@ -1,4 +1,5 @@ import { useCallback } from "react"; +import type { ReactNode } from "react"; import { Dialog, DialogContent, @@ -14,7 +15,7 @@ interface ConfirmDialogProps { open: boolean; onOpenChange: (open: boolean) => void; title: string; - description: string; + description: ReactNode; confirmLabel?: string; cancelLabel?: string; variant?: "default" | "destructive"; @@ -68,7 +69,9 @@ export function ConfirmDialog({ {title} - {description} + +
{description}
+ + {(action || onDismiss) && ( +
+ {action && ( + + )} + {onDismiss && ( + + )} +
)} ); diff --git a/src/pages/ServerCatalog.test.tsx b/src/pages/ServerCatalog.test.tsx index d72f339e..7c97d8cd 100644 --- a/src/pages/ServerCatalog.test.tsx +++ b/src/pages/ServerCatalog.test.tsx @@ -285,6 +285,30 @@ describe("ServerCatalog", () => { expect(await screen.findByText("Globalping disconnected.")).toBeInTheDocument(); }); + it("announces impact-preview loading to screen readers", async () => { + const user = userEvent.setup(); + let resolvePreview: (preview: { gatewayId: string; servers: never[] }) => void; + mockGetGatewayImpactPreview.mockReturnValue( + new Promise((resolve) => { + resolvePreview = resolve; + }), + ); + renderWithRouter(); + + await user.click(screen.getByRole("button", { name: "Actions for Globalping" })); + await user.click(screen.getByRole("menuitem", { name: "Disconnect" })); + + expect(screen.getByText("Checking affected virtual servers…")).toHaveAttribute( + "aria-live", + "polite", + ); + expect(screen.getByText("Checking affected virtual servers…")).toHaveAttribute( + "aria-atomic", + "true", + ); + resolvePreview!({ gatewayId: "gateway-globalping", servers: [] }); + }); + it("waits for catalog state after an async disconnect", async () => { const user = userEvent.setup(); const refetch = vi.fn().mockResolvedValue({ diff --git a/src/pages/ServerCatalog.tsx b/src/pages/ServerCatalog.tsx index e7c4fd8e..cb8ca6c0 100644 --- a/src/pages/ServerCatalog.tsx +++ b/src/pages/ServerCatalog.tsx @@ -47,9 +47,10 @@ interface CatalogFilters { interface RegistrationNotification { type: "success" | "error"; message: string; + retryCatalogServer?: CatalogServer; } -const DISCONNECT_POLL_TIMEOUT_MS = 60_000; +const DISCONNECT_POLL_TIMEOUT_MS = 30_000; const DEFAULT_DISCONNECT_POLL_MS = 1_000; class DisconnectPollTimeoutError extends Error { @@ -542,6 +543,55 @@ export function ServerCatalog() { [refetch], ); + const handleDisconnectStatusRetry = useCallback( + async (server: CatalogServer) => { + if (!beginDisconnecting(server.id)) return; + + let pollController: AbortController | null = null; + setRegistrationNotification(null); + try { + disconnectPollAbortRef.current?.abort(); + pollController = new AbortController(); + disconnectPollAbortRef.current = pollController; + await waitForCatalogDisconnect( + server.id, + DEFAULT_DISCONNECT_POLL_MS, + pollController.signal, + ); + shouldFocusRegistrationNotificationRef.current = true; + setRegistrationNotification({ + type: "success", + message: intl.formatMessage( + { id: "mcpServer.catalog.disconnectSuccess" }, + { name: server.name }, + ), + }); + } catch (error) { + if (isAbortError(error)) return; + shouldFocusRegistrationNotificationRef.current = true; + setRegistrationNotification({ + type: "error", + message: intl.formatMessage( + { + id: + error instanceof DisconnectPollTimeoutError + ? "mcpServer.catalog.disconnectPending" + : "mcpServer.catalog.disconnectError", + }, + { name: server.name }, + ), + ...(error instanceof DisconnectPollTimeoutError ? { retryCatalogServer: server } : {}), + }); + } finally { + if (disconnectPollAbortRef.current === pollController) { + disconnectPollAbortRef.current = null; + } + endDisconnecting(server.id); + } + }, + [beginDisconnecting, endDisconnecting, intl, waitForCatalogDisconnect], + ); + const confirmDisconnect = useCallback(async () => { const gatewayId = disconnectServer?.gateway_id; if (!disconnectServer || !gatewayId || !beginDisconnecting(disconnectServer.id)) { @@ -595,6 +645,7 @@ export function ServerCatalog() { }, { name: server.name }, ), + ...(error instanceof DisconnectPollTimeoutError ? { retryCatalogServer: server } : {}), }); } finally { if (disconnectPollAbortRef.current === pollController) { @@ -693,6 +744,17 @@ export function ServerCatalog() { ref={registrationNotificationRef} type={registrationNotification.type} message={registrationNotification.message} + action={ + registrationNotification.retryCatalogServer + ? { + label: intl.formatMessage({ id: "mcpServer.catalog.retry" }), + onClick: () => + void handleDisconnectStatusRetry( + registrationNotification.retryCatalogServer!, + ), + } + : undefined + } tabIndex={-1} onDismiss={handleRegistrationNotificationDismiss} /> @@ -729,7 +791,7 @@ export function ServerCatalog() { )}

{impactPreviewLoading && ( -

+

{intl.formatMessage({ id: "mcpServer.catalog.disconnectImpactLoading" })}

)} From b5fb5fe840fa1dc662be639b5ffd38124b8aee77 Mon Sep 17 00:00:00 2001 From: Vishu Bhatnagar Date: Wed, 26 Aug 2026 13:56:01 +0100 Subject: [PATCH 4/8] test: cover catalog disconnect retry Signed-off-by: Vishu Bhatnagar --- src/pages/ServerCatalog.test.tsx | 45 +++++++++++++++++++++++++++++++- 1 file changed, 44 insertions(+), 1 deletion(-) diff --git a/src/pages/ServerCatalog.test.tsx b/src/pages/ServerCatalog.test.tsx index 7c97d8cd..b98d59fa 100644 --- a/src/pages/ServerCatalog.test.tsx +++ b/src/pages/ServerCatalog.test.tsx @@ -1,6 +1,6 @@ import type { ReactElement } from "react"; import { beforeEach, describe, expect, it, vi } from "vitest"; -import { fireEvent, render, screen, waitFor, within } from "@testing-library/react"; +import { act, fireEvent, render, screen, waitFor, within } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { @@ -332,6 +332,49 @@ describe("ServerCatalog", () => { expect(await screen.findByText("Globalping disconnected.")).toBeInTheDocument(); }); + it("retries async disconnect status without sending DELETE again", async () => { + const user = userEvent.setup(); + const refetch = vi.fn().mockResolvedValue(response); + mockUseQuery.mockReturnValue(queryResult({ refetch })); + mockDisconnectCatalogGateway.mockClear(); + mockDisconnectCatalogGateway.mockResolvedValue({ + status: 202, + data: { status: "deleting" }, + headers: new Headers({ "Retry-After": "0.001" }), + }); + + renderWithRouter(); + await user.click(screen.getByRole("button", { name: "Actions for Globalping" })); + await user.click(screen.getByRole("menuitem", { name: "Disconnect" })); + const dialog = await screen.findByRole("alertdialog"); + + vi.useFakeTimers(); + try { + fireEvent.click(within(dialog).getByRole("button", { name: "Disconnect" })); + + await act(async () => { + await Promise.resolve(); + await vi.advanceTimersByTimeAsync(60_000); + }); + + expect(screen.getByText("Globalping is still disconnecting. Refresh shortly.")).toBeVisible(); + refetch.mockResolvedValue({ + ...response, + servers: [{ ...openConnected, is_registered: false, gateway_id: null }], + }); + fireEvent.click(screen.getByRole("button", { name: "Retry" })); + + await act(async () => { + await vi.advanceTimersByTimeAsync(1_000); + }); + + expect(screen.getByText("Globalping disconnected.")).toBeVisible(); + expect(mockDisconnectCatalogGateway).toHaveBeenCalledOnce(); + } finally { + vi.useRealTimers(); + } + }); + it("keeps catalog state unchanged when backend rejects disconnect ownership", async () => { const user = userEvent.setup(); const refetch = vi.fn(); From 5d194b00daac0deda603895859fd7589c481c7af Mon Sep 17 00:00:00 2001 From: Vishu Bhatnagar Date: Wed, 26 Aug 2026 14:33:41 +0100 Subject: [PATCH 5/8] test: cover catalog disconnect polling Signed-off-by: Vishu Bhatnagar --- src/pages/ServerCatalog.test.tsx | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/pages/ServerCatalog.test.tsx b/src/pages/ServerCatalog.test.tsx index b98d59fa..fb3f81e2 100644 --- a/src/pages/ServerCatalog.test.tsx +++ b/src/pages/ServerCatalog.test.tsx @@ -334,7 +334,10 @@ describe("ServerCatalog", () => { it("retries async disconnect status without sending DELETE again", async () => { const user = userEvent.setup(); - const refetch = vi.fn().mockResolvedValue(response); + const refetch = vi + .fn() + .mockRejectedValueOnce(new Error("Temporary catalog failure")) + .mockResolvedValue(response); mockUseQuery.mockReturnValue(queryResult({ refetch })); mockDisconnectCatalogGateway.mockClear(); mockDisconnectCatalogGateway.mockResolvedValue({ @@ -357,6 +360,13 @@ describe("ServerCatalog", () => { await vi.advanceTimersByTimeAsync(60_000); }); + expect(screen.getByText("Globalping is still disconnecting. Refresh shortly.")).toBeVisible(); + fireEvent.click(screen.getByRole("button", { name: "Retry" })); + + await act(async () => { + await vi.advanceTimersByTimeAsync(60_000); + }); + expect(screen.getByText("Globalping is still disconnecting. Refresh shortly.")).toBeVisible(); refetch.mockResolvedValue({ ...response, From 3b9d25eb90d6d601fb896c0a8d7ce20277c30e85 Mon Sep 17 00:00:00 2001 From: Vishu Bhatnagar Date: Thu, 27 Aug 2026 09:36:31 +0100 Subject: [PATCH 6/8] fix: harden catalog disconnect flow Signed-off-by: Vishu Bhatnagar --- src/i18n/locales/en-US/mcpServer.json | 4 +- src/i18n/locales/es-ES/mcpServer.json | 4 +- src/i18n/locales/pt-BR/mcpServer.json | 4 +- src/pages/ServerCatalog.test.tsx | 80 +++++- src/pages/ServerCatalog.tsx | 338 ++++++++++++++++---------- 5 files changed, 297 insertions(+), 133 deletions(-) diff --git a/src/i18n/locales/en-US/mcpServer.json b/src/i18n/locales/en-US/mcpServer.json index 2dd50276..c7976ac5 100644 --- a/src/i18n/locales/en-US/mcpServer.json +++ b/src/i18n/locales/en-US/mcpServer.json @@ -101,8 +101,8 @@ "mcpServer.catalog.viewDetails": "View details", "mcpServer.catalog.test": "Test connection", "mcpServer.catalog.testOAuthPending": "Complete OAuth configuration before testing this server.", - "mcpServer.catalog.testSuccess": "{name} responded with status {statusCode} in {latencyMs} ms.", - "mcpServer.catalog.testFailure": "{name} responded with status {statusCode} in {latencyMs} ms.", + "mcpServer.catalog.testSuccess": "{name} connection succeeded with status {statusCode} in {latencyMs} ms.", + "mcpServer.catalog.testFailure": "{name} connection failed with status {statusCode} in {latencyMs} ms.", "mcpServer.catalog.testError": "Unable to test {name}. Try again.", "mcpServer.catalog.disconnect": "Disconnect", "mcpServer.catalog.disconnecting": "Disconnecting…", diff --git a/src/i18n/locales/es-ES/mcpServer.json b/src/i18n/locales/es-ES/mcpServer.json index e27490e8..1bb8e1e8 100644 --- a/src/i18n/locales/es-ES/mcpServer.json +++ b/src/i18n/locales/es-ES/mcpServer.json @@ -101,8 +101,8 @@ "mcpServer.catalog.viewDetails": "Ver detalles", "mcpServer.catalog.test": "Probar conexión", "mcpServer.catalog.testOAuthPending": "Completa la configuración de OAuth antes de probar este servidor.", - "mcpServer.catalog.testSuccess": "{name} respondió con el estado {statusCode} en {latencyMs} ms.", - "mcpServer.catalog.testFailure": "{name} respondió con el estado {statusCode} en {latencyMs} ms.", + "mcpServer.catalog.testSuccess": "La conexión con {name} se realizó correctamente con el estado {statusCode} en {latencyMs} ms.", + "mcpServer.catalog.testFailure": "La conexión con {name} falló con el estado {statusCode} en {latencyMs} ms.", "mcpServer.catalog.testError": "No se pudo probar {name}. Inténtalo de nuevo.", "mcpServer.catalog.disconnect": "Desconectar", "mcpServer.catalog.disconnecting": "Desconectando…", diff --git a/src/i18n/locales/pt-BR/mcpServer.json b/src/i18n/locales/pt-BR/mcpServer.json index 85071ebb..04a9ade7 100644 --- a/src/i18n/locales/pt-BR/mcpServer.json +++ b/src/i18n/locales/pt-BR/mcpServer.json @@ -101,8 +101,8 @@ "mcpServer.catalog.viewDetails": "Ver detalhes", "mcpServer.catalog.test": "Testar conexão", "mcpServer.catalog.testOAuthPending": "Conclua a configuração do OAuth antes de testar este servidor.", - "mcpServer.catalog.testSuccess": "{name} respondeu com status {statusCode} em {latencyMs} ms.", - "mcpServer.catalog.testFailure": "{name} respondeu com status {statusCode} em {latencyMs} ms.", + "mcpServer.catalog.testSuccess": "A conexão com {name} foi bem-sucedida com status {statusCode} em {latencyMs} ms.", + "mcpServer.catalog.testFailure": "A conexão com {name} falhou com status {statusCode} em {latencyMs} ms.", "mcpServer.catalog.testError": "Não foi possível testar {name}. Tente novamente.", "mcpServer.catalog.disconnect": "Desconectar", "mcpServer.catalog.disconnecting": "Desconectando…", diff --git a/src/pages/ServerCatalog.test.tsx b/src/pages/ServerCatalog.test.tsx index fb3f81e2..2732f44f 100644 --- a/src/pages/ServerCatalog.test.tsx +++ b/src/pages/ServerCatalog.test.tsx @@ -20,7 +20,7 @@ vi.mock("@/hooks/useQuery", () => ({ useQuery: vi.fn(), })); const authState = vi.hoisted(() => ({ - hasPermission: vi.fn(() => true), + hasPermission: vi.fn<(permission: string) => boolean>(() => true), permissionsLoading: false, })); vi.mock("@/auth/useAuth", () => ({ @@ -225,7 +225,39 @@ describe("ServerCatalog", () => { await waitFor(() => expect(mockTestCatalogServer).toHaveBeenCalledWith("https://globalping.example/mcp"), ); - expect(await screen.findByText("Globalping responded with status 200 in 12 ms.")).toBeVisible(); + expect( + await screen.findByText("Globalping connection succeeded with status 200 in 12 ms."), + ).toBeVisible(); + }); + + it("reports a failed connection distinctly from a successful connection", async () => { + const user = userEvent.setup(); + mockTestCatalogServer.mockResolvedValue({ statusCode: 503, latencyMs: 12 }); + renderWithRouter(); + + await user.click(screen.getByRole("button", { name: "Actions for Globalping" })); + await user.click(screen.getByRole("menuitem", { name: "Test connection" })); + + expect( + await screen.findByText("Globalping connection failed with status 503 in 12 ms."), + ).toBeVisible(); + }); + + it("keeps notifications from independent server mutations", async () => { + const user = userEvent.setup(); + mockTestCatalogServer.mockResolvedValue({ statusCode: 503, latencyMs: 12 }); + mockRegisterCatalogServer.mockRejectedValue(new Error("Network error")); + renderWithRouter(); + + await user.click(screen.getByRole("button", { name: "Actions for Globalping" })); + await user.click(screen.getByRole("menuitem", { name: "Test connection" })); + await screen.findByText("Globalping connection failed with status 503 in 12 ms."); + + await user.click(screen.getByRole("button", { name: "Add Public Notes" })); + expect(await screen.findByText("Unable to add this server. Try again.")).toBeVisible(); + expect( + screen.getByText("Globalping connection failed with status 503 in 12 ms."), + ).toBeVisible(); }); it("disables Test when OAuth configuration remains incomplete", async () => { @@ -261,6 +293,20 @@ describe("ServerCatalog", () => { expect(screen.getByRole("menuitem", { name: "View details" })).toBeInTheDocument(); }); + it("loads disconnect impact for a caller allowed to delete but not test", async () => { + const user = userEvent.setup(); + authState.hasPermission.mockImplementation((permission) => permission === "gateways.delete"); + renderWithRouter(); + + await user.click(screen.getByRole("button", { name: "Actions for Globalping" })); + expect(screen.queryByRole("menuitem", { name: "Test connection" })).not.toBeInTheDocument(); + await user.click(screen.getByRole("menuitem", { name: "Disconnect" })); + + await waitFor(() => + expect(mockGetGatewayImpactPreview).toHaveBeenCalledWith("gateway-globalping"), + ); + }); + it("confirms disconnect, shows affected virtual servers, then refetches catalog", async () => { const user = userEvent.setup(); const refetch = vi.fn().mockResolvedValue(response); @@ -385,6 +431,36 @@ describe("ServerCatalog", () => { } }); + it("reports persistent catalog polling failures as errors", async () => { + const user = userEvent.setup(); + const refetch = vi.fn().mockRejectedValue(new ApiError(500, null, "HTTP 500")); + mockUseQuery.mockReturnValue(queryResult({ refetch })); + mockDisconnectCatalogGateway.mockResolvedValue({ + status: 202, + data: { status: "deleting" }, + headers: new Headers({ "Retry-After": "0.001" }), + }); + renderWithRouter(); + + await user.click(screen.getByRole("button", { name: "Actions for Globalping" })); + await user.click(screen.getByRole("menuitem", { name: "Disconnect" })); + const dialog = await screen.findByRole("alertdialog"); + + vi.useFakeTimers(); + try { + fireEvent.click(within(dialog).getByRole("button", { name: "Disconnect" })); + await act(async () => { + await Promise.resolve(); + await vi.advanceTimersByTimeAsync(60_000); + }); + + expect(screen.getByText("Unable to disconnect Globalping. Try again.")).toBeVisible(); + expect(refetch).toHaveBeenCalledTimes(3); + } finally { + vi.useRealTimers(); + } + }); + it("keeps catalog state unchanged when backend rejects disconnect ownership", async () => { const user = userEvent.setup(); const refetch = vi.fn(); diff --git a/src/pages/ServerCatalog.tsx b/src/pages/ServerCatalog.tsx index cb8ca6c0..a05cfed5 100644 --- a/src/pages/ServerCatalog.tsx +++ b/src/pages/ServerCatalog.tsx @@ -45,6 +45,7 @@ interface CatalogFilters { } interface RegistrationNotification { + id: string; type: "success" | "error"; message: string; retryCatalogServer?: CatalogServer; @@ -306,17 +307,18 @@ export function ServerCatalog() { end: endDisconnecting, isPending: isDisconnecting, } = usePendingIds(); - const [registrationNotification, setRegistrationNotification] = - useState(null); + const [registrationNotifications, setRegistrationNotifications] = useState< + RegistrationNotification[] + >([]); const [disconnectServer, setDisconnectServer] = useState(null); const [impactPreview, setImpactPreview] = useState(null); const [impactPreviewLoading, setImpactPreviewLoading] = useState(false); const impactRequestIdRef = useRef(0); - const disconnectPollAbortRef = useRef(null); + const disconnectPollAbortControllersRef = useRef(new Map()); const lastViewTriggerRef = useRef(null); const pageHeadingRef = useRef(null); - const registrationNotificationRef = useRef(null); - const shouldFocusRegistrationNotificationRef = useRef(false); + const registrationNotificationRefs = useRef(new Map()); + const notificationToFocusRef = useRef(null); const { data, error, isLoading, refetch, setData } = useQuery(CATALOG_PATH); const canTest = !permissionsLoading && hasPermission("gateways.read"); const canDisconnect = !permissionsLoading && hasPermission("gateways.delete"); @@ -336,20 +338,39 @@ export function ServerCatalog() { }, [debouncedSearch, filters.search, updateQuery]); useEffect(() => { - if (!registrationNotification || !shouldFocusRegistrationNotificationRef.current) return; + const notificationId = notificationToFocusRef.current; + if (!notificationId) return; - shouldFocusRegistrationNotificationRef.current = false; - registrationNotificationRef.current?.focus(); - }, [registrationNotification]); + notificationToFocusRef.current = null; + registrationNotificationRefs.current.get(notificationId)?.focus(); + }, [registrationNotifications]); useEffect( () => () => { impactRequestIdRef.current += 1; - disconnectPollAbortRef.current?.abort(); + disconnectPollAbortControllersRef.current.forEach((controller) => controller.abort()); + disconnectPollAbortControllersRef.current.clear(); }, [], ); + const showRegistrationNotification = useCallback( + (notification: RegistrationNotification, shouldFocus = false) => { + if (shouldFocus) notificationToFocusRef.current = notification.id; + setRegistrationNotifications((current) => [ + ...current.filter((existing) => existing.id !== notification.id), + notification, + ]); + }, + [], + ); + + const dismissRegistrationNotification = useCallback((notificationId: string) => { + setRegistrationNotifications((current) => + current.filter((notification) => notification.id !== notificationId), + ); + }, []); + // Only the search box filters ahead of the URL, so the grid can react to the // debounced value. Category, provider and tag selections are committed to the // URL the moment they are ticked in the filters popover. @@ -398,11 +419,12 @@ export function ServerCatalog() { const handleAdd = useCallback( async (server: CatalogServer) => { if (!beginAdding(server.id)) return; - setRegistrationNotification(null); + dismissRegistrationNotification(`add:${server.id}`); try { const result = await registerCatalogServer(server.id); if (!result.success) { - setRegistrationNotification({ + showRegistrationNotification({ + id: `add:${server.id}`, type: "error", message: result.message || intl.formatMessage({ id: "mcpServer.catalog.addError" }), }); @@ -415,7 +437,8 @@ export function ServerCatalog() { void refreshCatalogSilently(); } catch (registrationError) { if (registrationError instanceof ApiError && registrationError.status === 409) { - setRegistrationNotification({ + showRegistrationNotification({ + id: `add:${server.id}`, type: "success", message: intl.formatMessage( { id: "mcpServer.catalog.alreadyConnected" }, @@ -428,19 +451,23 @@ export function ServerCatalog() { if (registrationError instanceof ApiError && registrationError.status === 404) { setData((current) => removeCatalogServer(current, server.id)); - shouldFocusRegistrationNotificationRef.current = true; - setRegistrationNotification({ - type: "error", - message: intl.formatMessage( - { id: "mcpServer.catalog.addNotFound" }, - { name: server.name }, - ), - }); + showRegistrationNotification( + { + id: `add:${server.id}`, + type: "error", + message: intl.formatMessage( + { id: "mcpServer.catalog.addNotFound" }, + { name: server.name }, + ), + }, + true, + ); await refreshCatalogSilently(); return; } - setRegistrationNotification({ + showRegistrationNotification({ + id: `add:${server.id}`, type: "error", message: intl.formatMessage({ id: "mcpServer.catalog.addError" }), }); @@ -448,7 +475,15 @@ export function ServerCatalog() { endAdding(server.id); } }, - [beginAdding, endAdding, intl, refreshCatalogSilently, setData], + [ + beginAdding, + dismissRegistrationNotification, + endAdding, + intl, + refreshCatalogSilently, + setData, + showRegistrationNotification, + ], ); const handleTest = useCallback( @@ -457,33 +492,49 @@ export function ServerCatalog() { return; } - setRegistrationNotification(null); + dismissRegistrationNotification(`test:${server.id}`); try { const result = await testCatalogServer(server.url); const statusCode = result?.statusCode ?? 0; const succeeded = statusCode >= 200 && statusCode < 300; - shouldFocusRegistrationNotificationRef.current = true; - setRegistrationNotification({ - type: succeeded ? "success" : "error", - message: intl.formatMessage( - { - id: succeeded ? "mcpServer.catalog.testSuccess" : "mcpServer.catalog.testFailure", - }, - { name: server.name, statusCode, latencyMs: result?.latencyMs ?? 0 }, - ), - }); + showRegistrationNotification( + { + id: `test:${server.id}`, + type: succeeded ? "success" : "error", + message: intl.formatMessage( + { + id: succeeded ? "mcpServer.catalog.testSuccess" : "mcpServer.catalog.testFailure", + }, + { name: server.name, statusCode, latencyMs: result?.latencyMs ?? 0 }, + ), + }, + true, + ); } catch { - shouldFocusRegistrationNotificationRef.current = true; - setRegistrationNotification({ - type: "error", - message: intl.formatMessage({ id: "mcpServer.catalog.testError" }, { name: server.name }), - }); + showRegistrationNotification( + { + id: `test:${server.id}`, + type: "error", + message: intl.formatMessage( + { id: "mcpServer.catalog.testError" }, + { name: server.name }, + ), + }, + true, + ); } finally { endTesting(server.id); } }, - [beginTesting, endTesting, intl, isDisconnecting], + [ + beginTesting, + dismissRegistrationNotification, + endTesting, + intl, + isDisconnecting, + showRegistrationNotification, + ], ); const handleDisconnect = useCallback( @@ -493,7 +544,7 @@ export function ServerCatalog() { setDisconnectServer(server); setImpactPreview(null); const requestId = ++impactRequestIdRef.current; - if (!canTest) return; + if (!canDisconnect) return; setImpactPreviewLoading(true); void getGatewayImpactPreview(server.gateway_id) @@ -509,7 +560,7 @@ export function ServerCatalog() { if (impactRequestIdRef.current === requestId) setImpactPreviewLoading(false); }); }, - [canTest, isDisconnecting, isTesting], + [canDisconnect, isDisconnecting, isTesting], ); const handleDisconnectDialogOpenChange = useCallback((open: boolean) => { @@ -524,6 +575,7 @@ export function ServerCatalog() { async (catalogId: string, initialDelayMs: number, signal: AbortSignal) => { const deadline = Date.now() + DISCONNECT_POLL_TIMEOUT_MS; let delayMs = initialDelayMs; + let consecutivePollErrors = 0; while (Date.now() < deadline) { await delay(delayMs, signal); @@ -531,9 +583,18 @@ export function ServerCatalog() { const catalog = await refetch(); const server = catalog.servers.find((candidate) => candidate.id === catalogId); if (!server?.is_registered || !server.gateway_id) return; - } catch { + consecutivePollErrors = 0; + } catch (error) { + if (isAbortError(error)) throw error; + consecutivePollErrors += 1; + if ( + (error instanceof ApiError && error.status >= 400 && error.status < 500) || + consecutivePollErrors >= 3 + ) { + throw error; + } // Deletion has already been accepted. Keep polling through transient - // catalog failures instead of reporting it as a failed deletion. + // catalog failures, but surface persistent or authorization failures. } delayMs = DEFAULT_DISCONNECT_POLL_MS; } @@ -548,48 +609,60 @@ export function ServerCatalog() { if (!beginDisconnecting(server.id)) return; let pollController: AbortController | null = null; - setRegistrationNotification(null); + dismissRegistrationNotification(`disconnect:${server.id}`); try { - disconnectPollAbortRef.current?.abort(); pollController = new AbortController(); - disconnectPollAbortRef.current = pollController; + disconnectPollAbortControllersRef.current.set(server.id, pollController); await waitForCatalogDisconnect( server.id, DEFAULT_DISCONNECT_POLL_MS, pollController.signal, ); - shouldFocusRegistrationNotificationRef.current = true; - setRegistrationNotification({ - type: "success", - message: intl.formatMessage( - { id: "mcpServer.catalog.disconnectSuccess" }, - { name: server.name }, - ), - }); + showRegistrationNotification( + { + id: `disconnect:${server.id}`, + type: "success", + message: intl.formatMessage( + { id: "mcpServer.catalog.disconnectSuccess" }, + { name: server.name }, + ), + }, + true, + ); } catch (error) { if (isAbortError(error)) return; - shouldFocusRegistrationNotificationRef.current = true; - setRegistrationNotification({ - type: "error", - message: intl.formatMessage( - { - id: - error instanceof DisconnectPollTimeoutError - ? "mcpServer.catalog.disconnectPending" - : "mcpServer.catalog.disconnectError", - }, - { name: server.name }, - ), - ...(error instanceof DisconnectPollTimeoutError ? { retryCatalogServer: server } : {}), - }); + showRegistrationNotification( + { + id: `disconnect:${server.id}`, + type: "error", + message: intl.formatMessage( + { + id: + error instanceof DisconnectPollTimeoutError + ? "mcpServer.catalog.disconnectPending" + : "mcpServer.catalog.disconnectError", + }, + { name: server.name }, + ), + ...(error instanceof DisconnectPollTimeoutError ? { retryCatalogServer: server } : {}), + }, + true, + ); } finally { - if (disconnectPollAbortRef.current === pollController) { - disconnectPollAbortRef.current = null; + if (disconnectPollAbortControllersRef.current.get(server.id) === pollController) { + disconnectPollAbortControllersRef.current.delete(server.id); } endDisconnecting(server.id); } }, - [beginDisconnecting, endDisconnecting, intl, waitForCatalogDisconnect], + [ + beginDisconnecting, + dismissRegistrationNotification, + endDisconnecting, + intl, + showRegistrationNotification, + waitForCatalogDisconnect, + ], ); const confirmDisconnect = useCallback(async () => { @@ -604,9 +677,8 @@ export function ServerCatalog() { try { const response = await disconnectCatalogGateway(gatewayId); if (response.status === 202) { - disconnectPollAbortRef.current?.abort(); pollController = new AbortController(); - disconnectPollAbortRef.current = pollController; + disconnectPollAbortControllersRef.current.set(server.id, pollController); await waitForCatalogDisconnect( server.id, getRetryAfterMs(response.headers.get("Retry-After")), @@ -619,37 +691,43 @@ export function ServerCatalog() { impactRequestIdRef.current += 1; setDisconnectServer(null); setImpactPreview(null); - shouldFocusRegistrationNotificationRef.current = true; - setRegistrationNotification({ - type: "success", - message: intl.formatMessage( - { id: "mcpServer.catalog.disconnectSuccess" }, - { name: server.name }, - ), - }); + showRegistrationNotification( + { + id: `disconnect:${server.id}`, + type: "success", + message: intl.formatMessage( + { id: "mcpServer.catalog.disconnectSuccess" }, + { name: server.name }, + ), + }, + true, + ); } catch (error) { if (isAbortError(error)) return; if (error instanceof DisconnectPollTimeoutError) { setDisconnectServer(null); setImpactPreview(null); } - shouldFocusRegistrationNotificationRef.current = true; - setRegistrationNotification({ - type: "error", - message: intl.formatMessage( - { - id: - error instanceof DisconnectPollTimeoutError - ? "mcpServer.catalog.disconnectPending" - : "mcpServer.catalog.disconnectError", - }, - { name: server.name }, - ), - ...(error instanceof DisconnectPollTimeoutError ? { retryCatalogServer: server } : {}), - }); + showRegistrationNotification( + { + id: `disconnect:${server.id}`, + type: "error", + message: intl.formatMessage( + { + id: + error instanceof DisconnectPollTimeoutError + ? "mcpServer.catalog.disconnectPending" + : "mcpServer.catalog.disconnectError", + }, + { name: server.name }, + ), + ...(error instanceof DisconnectPollTimeoutError ? { retryCatalogServer: server } : {}), + }, + true, + ); } finally { - if (disconnectPollAbortRef.current === pollController) { - disconnectPollAbortRef.current = null; + if (disconnectPollAbortControllersRef.current.get(server.id) === pollController) { + disconnectPollAbortControllersRef.current.delete(server.id); } endDisconnecting(server.id); } @@ -660,6 +738,7 @@ export function ServerCatalog() { intl, refreshCatalogSilently, setData, + showRegistrationNotification, waitForCatalogDisconnect, ]); @@ -669,14 +748,17 @@ export function ServerCatalog() { window.setTimeout(() => lastViewTriggerRef.current?.focus(), 0); }, []); - const handleRegistrationNotificationDismiss = useCallback(() => { - const notification = registrationNotificationRef.current; - const shouldRestoreFocus = notification?.contains(notification.ownerDocument.activeElement); - setRegistrationNotification(null); - if (shouldRestoreFocus) { - window.setTimeout(() => pageHeadingRef.current?.focus(), 0); - } - }, []); + const handleRegistrationNotificationDismiss = useCallback( + (notificationId: string) => { + const notification = registrationNotificationRefs.current.get(notificationId); + const shouldRestoreFocus = notification?.contains(notification.ownerDocument.activeElement); + dismissRegistrationNotification(notificationId); + if (shouldRestoreFocus) { + window.setTimeout(() => pageHeadingRef.current?.focus(), 0); + } + }, + [dismissRegistrationNotification], + ); if (isLoading && !data) { return ( @@ -738,26 +820,32 @@ export function ServerCatalog() { onClearAll={clearAllFilters} /> - {registrationNotification && ( + {registrationNotifications.length > 0 && (
- - void handleDisconnectStatusRetry( - registrationNotification.retryCatalogServer!, - ), - } - : undefined - } - tabIndex={-1} - onDismiss={handleRegistrationNotificationDismiss} - /> +
+ {registrationNotifications.map((notification) => ( + { + if (element) registrationNotificationRefs.current.set(notification.id, element); + else registrationNotificationRefs.current.delete(notification.id); + }} + type={notification.type} + message={notification.message} + action={ + notification.retryCatalogServer + ? { + label: intl.formatMessage({ id: "mcpServer.catalog.retry" }), + onClick: () => + void handleDisconnectStatusRetry(notification.retryCatalogServer!), + } + : undefined + } + tabIndex={-1} + onDismiss={() => handleRegistrationNotificationDismiss(notification.id)} + /> + ))} +
)} From a42271eb0987ec620a31448df9ac17ca3841e960 Mon Sep 17 00:00:00 2001 From: Vishu Bhatnagar Date: Thu, 27 Aug 2026 14:29:05 +0100 Subject: [PATCH 7/8] fix: address catalog action review feedback Signed-off-by: Vishu Bhatnagar --- .../server-catalog/CatalogResults.test.tsx | 43 ++++++- .../server-catalog/CatalogResults.tsx | 36 ++++-- .../ui/inline-notification.test.tsx | 9 ++ src/components/ui/inline-notification.tsx | 19 ++- src/i18n/locales/en-US/mcpServer.json | 6 +- src/i18n/locales/es-ES/mcpServer.json | 6 +- src/i18n/locales/pt-BR/mcpServer.json | 6 +- src/pages/ServerCatalog.test.tsx | 75 +++++++++-- src/pages/ServerCatalog.tsx | 117 ++++++++++++++---- 9 files changed, 260 insertions(+), 57 deletions(-) diff --git a/src/components/server-catalog/CatalogResults.test.tsx b/src/components/server-catalog/CatalogResults.test.tsx index a19ff3a6..5adeb0ca 100644 --- a/src/components/server-catalog/CatalogResults.test.tsx +++ b/src/components/server-catalog/CatalogResults.test.tsx @@ -18,7 +18,12 @@ const availableServer: CatalogServer = { is_registered: false, }; -function catalogResults(server: CatalogServer, addingServerIds: ReadonlySet = new Set()) { +function catalogResults( + server: CatalogServer, + addingServerIds: ReadonlySet = new Set(), + testingServerIds: ReadonlySet = new Set(), + disconnectingServerIds: ReadonlySet = new Set(), +) { return ( @@ -68,4 +75,38 @@ describe("CatalogResults", () => { expect(await screen.findByRole("menu")).toHaveAttribute("data-align", "end"); }); + + it("shows testing and disconnecting status on the affected card", () => { + const connectedServer = { ...availableServer, is_registered: true, gateway_id: "gateway-1" }; + const { rerender } = renderWithProviders( + catalogResults(connectedServer, new Set(), new Set([connectedServer.id])), + ); + + expect(screen.getByText("Testing connection…")).toHaveAttribute("role", "status"); + + rerender(catalogResults(connectedServer, new Set(), new Set(), new Set([connectedServer.id]))); + + expect(screen.getByText("Disconnecting…")).toHaveAttribute("role", "status"); + expect(screen.queryByText("Connected")).not.toBeInTheDocument(); + }); + + it("routes bundled catalog logos through the BFF", () => { + const { container } = renderWithProviders( + catalogResults({ ...availableServer, logo_url: "/static/catalog-icons/asana.png" }), + ); + + const logo = container.querySelector("img"); + + expect(logo).toHaveAttribute("src", "/api/static/catalog-icons/asana.png"); + expect(logo).toHaveClass("size-full", "object-contain"); + expect(logo?.parentElement).not.toHaveClass("bg-muted"); + }); + + it("rejects local logo paths outside the catalog icon directory", () => { + const { container } = renderWithProviders( + catalogResults({ ...availableServer, logo_url: "/static/admin.png" }), + ); + + expect(container.querySelector("img")).not.toBeInTheDocument(); + }); }); diff --git a/src/components/server-catalog/CatalogResults.tsx b/src/components/server-catalog/CatalogResults.tsx index 8684cf41..1178c74d 100644 --- a/src/components/server-catalog/CatalogResults.tsx +++ b/src/components/server-catalog/CatalogResults.tsx @@ -1,6 +1,6 @@ import { useEffect, useId, useRef, useState } from "react"; import type { ReactNode } from "react"; -import { CircleCheck, EllipsisVertical, FileText, PlugZap, Plus, Unplug } from "lucide-react"; +import { CircleCheck, EllipsisVertical, FileText, Plus } from "lucide-react"; import { useIntl } from "react-intl"; import { EmptyStatePlaceholder } from "@/components/dashboard/EmptyStatePlaceholder"; @@ -26,10 +26,17 @@ import { useDebouncedValue } from "@/hooks/useDebouncedValue"; import { getTagLabels } from "@/utils/tags"; const EMPTY_PENDING_IDS: ReadonlySet = new Set(); +const CATALOG_ICON_PATH = /^\/static\/catalog-icons\/[A-Za-z0-9][A-Za-z0-9._-]*\.png$/; function getSafeExternalUrl(value: string | null | undefined): string | null { if (!value) return null; + // Catalog icons are packaged by the API under this fixed path. Route them + // through the authenticated BFF so the browser never needs an API origin. + if (CATALOG_ICON_PATH.test(value)) { + return `/api${value}`; + } + try { const parsed = new URL(value); return parsed.protocol === "https:" && !parsed.username && !parsed.password @@ -53,14 +60,11 @@ function CatalogLogo({ server }: { server: CatalogServer }) { } return ( -