From efc0fea910e56f0a1297595f1d0ba1135fce641d Mon Sep 17 00:00:00 2001 From: Anna Effort Date: Mon, 31 Aug 2026 18:43:26 -0700 Subject: [PATCH 1/7] feat: register Quick Add picks through the catalog endpoint Selecting a server in the Quick Add dialog only prefilled the connect form and left the user to submit it. It now registers via /v1/catalog/{id}/register and goes straight to the detected components step, so the form is skipped. Pending and failure state stay in the dialog. Drops the form prefill plumbing, which no longer has a caller. Signed-off-by: Anna Effort --- e2e/quick-add-server.spec.ts | 85 ++++++---- .../mcp-servers/MCPServerForm.test.tsx | 49 ++++-- src/components/mcp-servers/MCPServerForm.tsx | 29 +--- .../mcp-servers/QuickAddServerDialog.test.tsx | 153 ++++++++++++++++-- .../mcp-servers/QuickAddServerDialog.tsx | 81 ++++++++-- src/config/quickAddServers.ts | 4 +- src/hooks/useMCPServerForm.test.ts | 47 +----- src/hooks/useMCPServerForm.ts | 25 +-- src/i18n/locales/en-US/mcpServer.json | 5 +- src/i18n/locales/es-ES/mcpServer.json | 5 +- src/i18n/locales/pt-BR/mcpServer.json | 5 +- 11 files changed, 323 insertions(+), 165 deletions(-) diff --git a/e2e/quick-add-server.spec.ts b/e2e/quick-add-server.spec.ts index aa4d5b65..c37d56da 100644 --- a/e2e/quick-add-server.spec.ts +++ b/e2e/quick-add-server.spec.ts @@ -41,6 +41,36 @@ async function mockCatalog(page: import("@playwright/test").Page, servers: Catal }); } +async function mockRegister( + page: import("@playwright/test").Page, + { status = 200 }: { status?: number } = {}, +) { + await page.route("**/v1/catalog/*/register", async (route) => { + await route.fulfill({ + status, + contentType: "application/json", + body: JSON.stringify( + status === 200 + ? { success: true, server_id: "new-gateway-1", message: "registered" } + : { detail: "boom" }, + ), + }); + }); +} + +// The detected-components step lists the gateway's tools, resources, and prompts. +async function mockComponentLists(page: import("@playwright/test").Page) { + for (const resource of ["tools", "resources", "prompts"]) { + await page.route(`**/${resource}?*`, async (route) => { + await route.fulfill({ + status: 200, + contentType: "application/json", + body: JSON.stringify([]), + }); + }); + } +} + async function openQuickAddDialog(page: import("@playwright/test").Page) { await page.route("**/gateways?*", async (route) => { await route.fulfill({ @@ -70,10 +100,12 @@ test.describe("Quick Add server dialog", () => { }); }); - test("pre-fills the connect form from a picked catalog entry and submits a new gateway", async ({ + test("registers a picked catalog entry and lands on the detected components step", async ({ page, }) => { await mockCatalog(page, [DEEPWIKI, EXA_SEARCH]); + await mockComponentLists(page); + await mockRegister(page); await openQuickAddDialog(page); // Only the curated entries render, in the configured order. @@ -88,38 +120,37 @@ test.describe("Quick Add server dialog", () => { await page.getByText("DeepWiki", { exact: true }).click(); await expect(page.getByRole("radio", { name: /DeepWiki/i })).toBeChecked(); await expect(continueButton).toBeEnabled(); + + const registerRequest = page.waitForRequest( + (request) => + request.url().includes("/v1/catalog/deepwiki/register") && request.method() === "POST", + ); await continueButton.click(); + await registerRequest; + // The connect form is skipped: Quick Add registers through the catalog endpoint. + await expect( + page.getByRole("heading", { name: "Expose MCP tools, resources, and prompts" }), + ).toBeVisible(); await expect(page.getByRole("dialog")).not.toBeVisible(); - await expect(page.getByLabel(/Name/i)).toHaveValue("DeepWiki"); - await expect(page.getByLabel(/URL/i)).toHaveValue("https://mcp.deepwiki.com/mcp"); - await expect(page.getByPlaceholder(/Add an optional description/i)).toHaveValue( - "Knowledge base with deep learning integration", - ); - await expect(page.getByRole("radio", { name: "Streamable HTTP" })).toBeChecked(); + await expect(page.getByLabel(/URL/i)).not.toBeVisible(); + }); - const createRequest = page.waitForRequest( - (request) => request.url().includes("/gateways") && request.method() === "POST", - ); - await page.route( - (url) => url.pathname.endsWith("/gateways") || url.pathname.endsWith("/api/gateways"), - async (route) => { - if (route.request().method() !== "POST") return route.fallback(); - await route.fulfill({ - status: 200, - contentType: "application/json", - body: JSON.stringify({ id: "new-gateway-1", name: "DeepWiki" }), - }); - }, - ); + test("keeps the dialog open and reports the failure when registration fails", async ({ + page, + }) => { + await mockCatalog(page, [DEEPWIKI, EXA_SEARCH]); + await mockRegister(page, { status: 500 }); + await openQuickAddDialog(page); - await page.getByRole("button", { name: /Connect server/i }).click(); + await page.getByText("DeepWiki", { exact: true }).click(); + await page.getByRole("button", { name: "Continue" }).click(); - const request = await createRequest; - const body = request.postDataJSON() as { name?: string; url?: string; transport?: string }; - expect(body.name).toBe("DeepWiki"); - expect(body.url).toBe("https://mcp.deepwiki.com/mcp"); - expect(body.transport).toBe("STREAMABLEHTTP"); + await expect(page.getByText("Unable to connect this server. Try again.")).toBeVisible(); + await expect(page.getByRole("dialog")).toBeVisible(); + await expect( + page.getByRole("heading", { name: "Expose MCP tools, resources, and prompts" }), + ).not.toBeVisible(); }); test("Browse full catalog closes the connect form and navigates to the full catalog", async ({ diff --git a/src/components/mcp-servers/MCPServerForm.test.tsx b/src/components/mcp-servers/MCPServerForm.test.tsx index c9cd57ff..5b945753 100644 --- a/src/components/mcp-servers/MCPServerForm.test.tsx +++ b/src/components/mcp-servers/MCPServerForm.test.tsx @@ -14,21 +14,18 @@ let mockHookReturnValue: Record | null = null; vi.mock("@/hooks/useMCPServerForm", async (importOriginal) => { const actual = (await importOriginal()) as { - useMCPServerForm: ( - serverId?: string, - initialValues?: Record, - ) => Record; + useMCPServerForm: (serverId?: string) => Record; }; return { ...actual, - useMCPServerForm: (serverId?: string, initialValues?: Record) => { + useMCPServerForm: (serverId?: string) => { if (mockHookActive) { return { - ...actual.useMCPServerForm(serverId, initialValues), + ...actual.useMCPServerForm(serverId), ...mockHookReturnValue, }; } - return actual.useMCPServerForm(serverId, initialValues); + return actual.useMCPServerForm(serverId); }, }; }); @@ -69,7 +66,15 @@ const server = setupServer( teams: [{ id: "team-personal", name: "Personal team", is_personal: true }], }); }), - // Quick Add dialog's catalog fetch — one curated entry is enough to exercise selection/prefill. + // Quick Add registers the picked entry through the catalog endpoint rather than the form. + http.post("/api/v1/catalog/:catalogId/register", () => { + return HttpResponse.json({ + success: true, + server_id: "quick-add-gateway-1", + message: "registered", + }); + }), + // Quick Add dialog's catalog fetch. http.get("/api/v1/catalog", () => { return HttpResponse.json({ servers: [ @@ -1242,7 +1247,7 @@ describe("MCPServerForm", () => { expect(window.location.pathname).toBe("/app/server-catalog"); }); - it("opens the dialog from the catalog link and pre-fills the form on selection", async () => { + it("opens the dialog from the catalog link and advances to the detected components step on selection", async () => { const user = userEvent.setup(); renderWithRouter(); @@ -1255,16 +1260,20 @@ describe("MCPServerForm", () => { await user.click(screen.getByRole("radio", { name: /DeepWiki/i })); await user.click(screen.getByRole("button", { name: "Continue" })); + // The connect form is skipped entirely: the gateway already exists by this point. + expect( + await screen.findByRole("heading", { name: "Expose MCP tools, resources, and prompts" }), + ).toBeInTheDocument(); expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); - expect(screen.getByLabelText(/Name/i)).toHaveValue("DeepWiki"); - expect(screen.getByLabelText(/URL/i)).toHaveValue("https://mcp.deepwiki.com/mcp"); - expect(screen.getByPlaceholderText(/Add an optional description/i)).toHaveValue( - "Knowledge base with deep learning integration", - ); - expect(screen.getByRole("radio", { name: "Streamable HTTP" })).toBeChecked(); + expect(screen.queryByLabelText(/URL/i)).not.toBeInTheDocument(); }); - it("maps a catalog entry's declared SSE transport onto the transport radio", async () => { + it("keeps the dialog open and reports the failure when registration fails", async () => { + server.use( + http.post("/api/v1/catalog/:catalogId/register", () => { + return new HttpResponse(null, { status: 500 }); + }), + ); const user = userEvent.setup(); renderWithRouter(); @@ -1272,7 +1281,13 @@ describe("MCPServerForm", () => { await user.click(screen.getByRole("radio", { name: /Exa Search/i })); await user.click(screen.getByRole("button", { name: "Continue" })); - expect(screen.getByRole("radio", { name: "SSE" })).toBeChecked(); + expect( + await screen.findByText("Unable to connect this server. Try again."), + ).toBeInTheDocument(); + expect(screen.getByRole("dialog")).toBeInTheDocument(); + expect( + screen.queryByRole("heading", { name: "Expose MCP tools, resources, and prompts" }), + ).not.toBeInTheDocument(); }); it("navigates to the full catalog and closes the form when Browse full catalog is clicked", async () => { diff --git a/src/components/mcp-servers/MCPServerForm.tsx b/src/components/mcp-servers/MCPServerForm.tsx index 501002c2..9b0217d1 100644 --- a/src/components/mcp-servers/MCPServerForm.tsx +++ b/src/components/mcp-servers/MCPServerForm.tsx @@ -10,19 +10,8 @@ import { AdvancedSettings } from "@/components/mcp-servers/AdvancedSettings"; import { QuickAddServerDialog } from "@/components/mcp-servers/QuickAddServerDialog"; import { ExposeComponentsForm } from "@/components/gateways/ExposeComponentsForm"; import { useRouter } from "@/router"; -import { - useMCPServerForm, - type MCPServerFormInitialValues, - type TransportType, -} from "@/hooks/useMCPServerForm"; +import { useMCPServerForm, type TransportType } from "@/hooks/useMCPServerForm"; import { STATUS_ICON } from "@/lib/status"; -import type { CatalogServer } from "@/generated/types"; - -// QuickAddServerDialog only surfaces entries with SSE, STREAMABLEHTTP, or no -// transport set, so anything else here defaults to STREAMABLEHTTP. -function mapCatalogTransport(transport: string | null | undefined): TransportType { - return transport === "SSE" ? "SSE" : "STREAMABLEHTTP"; -} interface MCPServerFormProps { isOpen: boolean; @@ -41,7 +30,6 @@ export function MCPServerForm({ isOpen, onToggle, serverId, onSuccess }: MCPServ const { navigate } = useRouter(); const [createdGateway, setCreatedGateway] = useState(null); const [quickAddOpen, setQuickAddOpen] = useState(false); - const [prefill, setPrefill] = useState(); const { fetchError, name, @@ -110,7 +98,7 @@ export function MCPServerForm({ isOpen, onToggle, serverId, onSuccess }: MCPServ setQueryParamName, queryParamApiKey, setQueryParamApiKey, - } = useMCPServerForm(serverId, prefill); + } = useMCPServerForm(serverId); const handleRedirectUriChange = useCallback( (uri: string) => { @@ -124,13 +112,10 @@ export function MCPServerForm({ isOpen, onToggle, serverId, onSuccess }: MCPServ onToggle(); }; - const handleQuickAddSelect = useCallback((server: CatalogServer) => { - setPrefill({ - name: server.name, - url: server.url, - description: server.description, - transport: mapCatalogTransport(server.transport), - }); + // Quick Add registers through the catalog endpoint, so the gateway already exists by the + // time this runs and the connect form is skipped entirely. + const handleQuickAddConnected = useCallback((gatewayId: string, serverName: string) => { + setCreatedGateway({ id: gatewayId, name: serverName }); setQuickAddOpen(false); }, []); @@ -485,7 +470,7 @@ export function MCPServerForm({ isOpen, onToggle, serverId, onSuccess }: MCPServ diff --git a/src/components/mcp-servers/QuickAddServerDialog.test.tsx b/src/components/mcp-servers/QuickAddServerDialog.test.tsx index c5d51508..de176814 100644 --- a/src/components/mcp-servers/QuickAddServerDialog.test.tsx +++ b/src/components/mcp-servers/QuickAddServerDialog.test.tsx @@ -1,7 +1,9 @@ -import { describe, expect, it, vi } from "vitest"; -import { screen } from "@testing-library/react"; +import { describe, expect, it, vi, beforeEach } from "vitest"; +import { screen, waitFor } 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 { renderWithProviders } from "@/test/test-utils"; @@ -12,7 +14,12 @@ vi.mock("@/hooks/useQuery", () => ({ useQuery: vi.fn(), })); +vi.mock("@/api/catalog", () => ({ + registerCatalogServer: vi.fn(), +})); + const mockUseQuery = vi.mocked(useQuery); +const mockRegister = vi.mocked(registerCatalogServer); function catalogServer( overrides: Partial & Pick, @@ -68,14 +75,22 @@ function mockCatalogQuery(overrides: Partial> = {}) } as ReturnType); } +function selectFirstCuratedCard(user: ReturnType) { + return user.click(screen.getByRole("radio", { name: new RegExp(QUICK_ADD_CATALOG_IDS[0]) })); +} + describe("QuickAddServerDialog", () => { + beforeEach(() => { + mockRegister.mockReset(); + }); + it("renders only the curated catalog entries", () => { mockCatalogQuery(); renderWithProviders( , ); @@ -91,7 +106,7 @@ describe("QuickAddServerDialog", () => { , ); @@ -106,7 +121,7 @@ describe("QuickAddServerDialog", () => { , ); @@ -115,15 +130,20 @@ describe("QuickAddServerDialog", () => { expect(screen.getByText(QUICK_ADD_CATALOG_IDS[1])).toBeInTheDocument(); }); - it("disables Continue until a card is selected, then calls onSelect with the picked server", async () => { + it("disables Continue until a card is selected, then registers and reports the new gateway id", async () => { mockCatalogQuery(); + mockRegister.mockResolvedValue({ + success: true, + server_id: "gateway-1", + message: "registered", + }); const user = userEvent.setup(); - const onSelect = vi.fn(); + const onConnected = vi.fn(); renderWithProviders( , ); @@ -131,32 +151,131 @@ describe("QuickAddServerDialog", () => { const continueButton = screen.getByRole("button", { name: "Continue" }); expect(continueButton).toBeDisabled(); - await user.click(screen.getByRole("radio", { name: new RegExp(QUICK_ADD_CATALOG_IDS[0]) })); + await selectFirstCuratedCard(user); expect(continueButton).toBeEnabled(); await user.click(continueButton); - expect(onSelect).toHaveBeenCalledWith( - expect.objectContaining({ id: QUICK_ADD_CATALOG_IDS[0] }), + + expect(mockRegister).toHaveBeenCalledWith(QUICK_ADD_CATALOG_IDS[0]); + await waitFor(() => { + expect(onConnected).toHaveBeenCalledWith("gateway-1", QUICK_ADD_CATALOG_IDS[0]); + }); + }); + + it("skips registration when the picked entry is already connected", async () => { + mockCatalogQuery({ + data: catalogResponseWith({ is_registered: true, gateway_id: "existing-gateway" }), + }); + const user = userEvent.setup(); + const onConnected = vi.fn(); + renderWithProviders( + , + ); + + await selectFirstCuratedCard(user); + await user.click(screen.getByRole("button", { name: "Continue" })); + + expect(mockRegister).not.toHaveBeenCalled(); + expect(onConnected).toHaveBeenCalledWith("existing-gateway", QUICK_ADD_CATALOG_IDS[0]); + }); + + it("keeps the dialog open and shows the failure when registration fails", async () => { + mockCatalogQuery(); + mockRegister.mockRejectedValue(new Error("boom")); + const user = userEvent.setup(); + const onConnected = vi.fn(); + renderWithProviders( + , ); + + await selectFirstCuratedCard(user); + await user.click(screen.getByRole("button", { name: "Continue" })); + + expect( + await screen.findByText("Unable to connect this server. Try again."), + ).toBeInTheDocument(); + expect(onConnected).not.toHaveBeenCalled(); + expect(screen.getByRole("button", { name: "Continue" })).toBeEnabled(); + }); + + it("points at the catalog when registration 409s on an entry the loaded list still shows as new", async () => { + mockCatalogQuery(); + mockRegister.mockRejectedValue(new ApiError(409, null, "conflict")); + const user = userEvent.setup(); + const onConnected = vi.fn(); + renderWithProviders( + , + ); + + await selectFirstCuratedCard(user); + await user.click(screen.getByRole("button", { name: "Continue" })); + + expect( + await screen.findByText( + `${QUICK_ADD_CATALOG_IDS[0]} is already connected. Manage it from the server catalog.`, + ), + ).toBeInTheDocument(); + expect(onConnected).not.toHaveBeenCalled(); + }); + + it("surfaces the backend message when registration reports failure", async () => { + mockCatalogQuery(); + mockRegister.mockResolvedValue({ + success: false, + server_id: "", + message: "Catalog entry is unavailable", + }); + const user = userEvent.setup(); + const onConnected = vi.fn(); + renderWithProviders( + , + ); + + await selectFirstCuratedCard(user); + await user.click(screen.getByRole("button", { name: "Continue" })); + + expect(await screen.findByText("Catalog entry is unavailable")).toBeInTheDocument(); + expect(onConnected).not.toHaveBeenCalled(); }); - it("closes without selecting when Cancel is clicked", async () => { + it("closes without connecting when Cancel is clicked", async () => { mockCatalogQuery(); const user = userEvent.setup(); const onOpenChange = vi.fn(); - const onSelect = vi.fn(); + const onConnected = vi.fn(); renderWithProviders( , ); await user.click(screen.getByRole("button", { name: "Cancel" })); expect(onOpenChange).toHaveBeenCalledWith(false); - expect(onSelect).not.toHaveBeenCalled(); + expect(onConnected).not.toHaveBeenCalled(); + expect(mockRegister).not.toHaveBeenCalled(); }); it("calls onBrowseCatalog when the browse-catalog link is clicked", async () => { @@ -167,7 +286,7 @@ describe("QuickAddServerDialog", () => { , ); @@ -182,7 +301,7 @@ describe("QuickAddServerDialog", () => { , ); diff --git a/src/components/mcp-servers/QuickAddServerDialog.tsx b/src/components/mcp-servers/QuickAddServerDialog.tsx index 801afb3a..809bf7a0 100644 --- a/src/components/mcp-servers/QuickAddServerDialog.tsx +++ b/src/components/mcp-servers/QuickAddServerDialog.tsx @@ -1,6 +1,8 @@ -import { useEffect, useId, useMemo, useState, type ReactNode } from "react"; +import { useCallback, useEffect, useId, useMemo, useState, type ReactNode } from "react"; import { useIntl } from "react-intl"; +import { registerCatalogServer } from "@/api/catalog"; +import { ApiError } from "@/api/client"; import { MCPIcon } from "@/components/icons/MCPIcon"; import { CatalogLogo } from "@/components/server-catalog/CatalogLogo"; import { Button } from "@/components/ui/button"; @@ -20,8 +22,8 @@ import type { CatalogListResponse, CatalogServer } from "@/generated/types"; import { useQuery } from "@/hooks/useQuery"; const CATALOG_PATH = "/v1/catalog?limit=1000"; -// Quick Add submits through the standard gateway-create form, which can't yet -// complete an OAuth setup flow, and only supports these two transports. +// Quick Add registers without collecting any credentials, so it can't complete an +// OAuth setup flow, and the gateway only supports these two transports. const OPEN_AUTH_TYPE = "Open"; const SUPPORTED_TRANSPORTS: ReadonlySet = new Set(["SSE", "STREAMABLEHTTP"]); @@ -36,27 +38,36 @@ function isQuickAddEligible(server: CatalogServer | undefined): server is Catalo interface QuickAddServerDialogProps { open: boolean; onOpenChange: (open: boolean) => void; - /** Called with the picked catalog entry. The caller is responsible for closing the dialog. */ - onSelect: (server: CatalogServer) => void; + /** + * Called once the picked entry is registered and a gateway exists for it. The caller is + * responsible for closing the dialog. + */ + onConnected: (gatewayId: string, serverName: string) => void; onBrowseCatalog: () => void; } export function QuickAddServerDialog({ open, onOpenChange, - onSelect, + onConnected, onBrowseCatalog, }: QuickAddServerDialogProps) { const intl = useIntl(); const groupLabelId = useId(); const [selectedId, setSelectedId] = useState(null); + const [isConnecting, setIsConnecting] = useState(false); + const [connectError, setConnectError] = useState(null); const { data, error, isLoading } = useQuery(CATALOG_PATH, { enabled: open, }); useEffect(() => { - if (!open) setSelectedId(null); + if (!open) { + setSelectedId(null); + setIsConnecting(false); + setConnectError(null); + } }, [open]); const servers = useMemo(() => { @@ -67,6 +78,42 @@ export function QuickAddServerDialog({ const selectedServer = servers.find((server) => server.id === selectedId) ?? null; + const handleContinue = useCallback(async () => { + if (!selectedServer || isConnecting) return; + setConnectError(null); + + // Already registered, so skip the round trip and go straight to the components step. + if (selectedServer.is_registered && selectedServer.gateway_id) { + onConnected(selectedServer.gateway_id, selectedServer.name); + return; + } + + setIsConnecting(true); + try { + const result = await registerCatalogServer(selectedServer.id); + if (!result.success || !result.server_id) { + setConnectError( + result.message || intl.formatMessage({ id: "mcpServer.quickAdd.connectError" }), + ); + return; + } + onConnected(result.server_id, selectedServer.name); + } catch (registrationError) { + // A 409 means the catalog list this dialog loaded has gone stale, so there is no + // gateway id to hand on. Point at the catalog rather than retrying into the same 409. + setConnectError( + registrationError instanceof ApiError && registrationError.status === 409 + ? intl.formatMessage( + { id: "mcpServer.quickAdd.alreadyConnected" }, + { name: selectedServer.name }, + ) + : intl.formatMessage({ id: "mcpServer.quickAdd.connectError" }), + ); + } finally { + setIsConnecting(false); + } + }, [intl, isConnecting, onConnected, selectedServer]); + return ( @@ -99,10 +146,13 @@ export function QuickAddServerDialog({

)} + {connectError && } + {servers.length > 0 && ( @@ -153,17 +203,22 @@ export function QuickAddServerDialog({ )}

-
diff --git a/src/config/quickAddServers.ts b/src/config/quickAddServers.ts index 71fc23ec..1d1af4d5 100644 --- a/src/config/quickAddServers.ts +++ b/src/config/quickAddServers.ts @@ -1,8 +1,8 @@ /** * Curated shortlist of catalog server ids shown in the Quick Add dialog * (issue #4681). Every id must resolve to an `auth_type: "Open"` entry in - * the backend's `mcp-catalog.yml`, since Quick Add submits through the - * standard gateway-create form and can't yet complete an OAuth setup flow + * the backend's `mcp-catalog.yml`, since Quick Add registers without collecting + * any credentials and can't yet complete an OAuth setup flow * (blocked on https://github.com/IBM/mcp-context-forge/issues/5967). * * Order here is the display order in the dialog grid. diff --git a/src/hooks/useMCPServerForm.test.ts b/src/hooks/useMCPServerForm.test.ts index 64d91be9..9146ca84 100644 --- a/src/hooks/useMCPServerForm.test.ts +++ b/src/hooks/useMCPServerForm.test.ts @@ -3,7 +3,7 @@ import { renderHook, act, waitFor } from "@testing-library/react"; import { http, HttpResponse } from "msw"; import { server } from "@/test/mocks/server"; import { serversApi } from "@/api/servers"; -import { useMCPServerForm, type MCPServerFormInitialValues } from "./useMCPServerForm"; +import { useMCPServerForm } from "./useMCPServerForm"; describe("useMCPServerForm", () => { describe("Initial State", () => { @@ -1707,47 +1707,14 @@ describe("useMCPServerForm", () => { }); }); - describe("initialValues (Quick Add prefill)", () => { - it("seeds create-mode fields from initialValues", () => { - const { result } = renderHook(() => - useMCPServerForm(undefined, { - name: "DeepWiki", - url: "https://mcp.deepwiki.com/mcp", - description: "Knowledge base with deep learning integration", - transport: "SSE", - }), - ); - - expect(result.current.name).toBe("DeepWiki"); - expect(result.current.url).toBe("https://mcp.deepwiki.com/mcp"); - expect(result.current.description).toBe("Knowledge base with deep learning integration"); - expect(result.current.transport).toBe("SSE"); - }); - - it("applies a later initialValues object once it arrives (Quick Add picked after the form opened)", () => { - const { result, rerender } = renderHook( - ({ initialValues }: { initialValues?: MCPServerFormInitialValues }) => - useMCPServerForm(undefined, initialValues), - { initialProps: { initialValues: undefined as MCPServerFormInitialValues | undefined } }, - ); + describe("create mode", () => { + it("starts with empty fields, since Quick Add registers through the catalog instead of seeding the form", () => { + const { result } = renderHook(() => useMCPServerForm()); expect(result.current.name).toBe(""); - - rerender({ initialValues: { name: "DeepWiki", url: "https://mcp.deepwiki.com/mcp" } }); - - expect(result.current.name).toBe("DeepWiki"); - expect(result.current.url).toBe("https://mcp.deepwiki.com/mcp"); - }); - - it("does not apply initialValues in edit mode", async () => { - const { result } = renderHook(() => - useMCPServerForm("edit-123", { name: "Should not apply" }), - ); - - await waitFor(() => { - expect(result.current.name).toBe("Test Server"); - }); - expect(result.current.name).not.toBe("Should not apply"); + expect(result.current.url).toBe(""); + expect(result.current.description).toBe(""); + expect(result.current.transport).toBe("STREAMABLEHTTP"); }); }); }); diff --git a/src/hooks/useMCPServerForm.ts b/src/hooks/useMCPServerForm.ts index 06492b19..a95a1538 100644 --- a/src/hooks/useMCPServerForm.ts +++ b/src/hooks/useMCPServerForm.ts @@ -17,14 +17,6 @@ import { export type TransportType = "SSE" | "STREAMABLEHTTP"; export type AuthType = "none" | "basic" | "bearer" | "custom" | "oauth" | "query"; -/** Seeds a freshly-opened create-mode form, e.g. from a picked Quick Add catalog entry. */ -export interface MCPServerFormInitialValues { - name?: string; - url?: string; - description?: string; - transport?: TransportType; -} - export interface CustomHeader { id: string; key: string; @@ -308,10 +300,7 @@ const initialState = { queryParamApiKey: "", // pragma: allowlist secret }; -export function useMCPServerForm( - gatewayId?: string, - initialValues?: MCPServerFormInitialValues, -): UseMCPServerFormReturn { +export function useMCPServerForm(gatewayId?: string): UseMCPServerFormReturn { const [name, setName] = useState(initialState.name); const [url, setUrl] = useState(initialState.url); const [description, setDescription] = useState(initialState.description); @@ -466,18 +455,6 @@ export function useMCPServerForm( } }, [serverData, gatewayId]); - // Seeds a freshly-opened create-mode form from caller-supplied defaults (e.g. a - // picked Quick Add catalog entry). Runs once per new initialValues reference — - // the caller is expected to hand in a new object only when a fresh pick is made, - // not on every render. Edit mode owns its own prefill via the effect above. - useEffect(() => { - if (!initialValues || gatewayId) return; - if (initialValues.name !== undefined) setName(initialValues.name); - if (initialValues.url !== undefined) setUrl(initialValues.url); - if (initialValues.description !== undefined) setDescription(initialValues.description); - if (initialValues.transport !== undefined) setTransport(initialValues.transport); - }, [initialValues, gatewayId]); - // Use useQuery for POST request to create MCP gateway const { execute: createGateway, isLoading: isCreating } = useQuery( "/gateways", diff --git a/src/i18n/locales/en-US/mcpServer.json b/src/i18n/locales/en-US/mcpServer.json index 3316e127..1fdeccc4 100644 --- a/src/i18n/locales/en-US/mcpServer.json +++ b/src/i18n/locales/en-US/mcpServer.json @@ -32,13 +32,16 @@ "mcpServer.form.saveChanges": "Save changes", "mcpServer.form.connectServer": "Connect server", "mcpServer.quickAdd.dialogTitle": "Connect MCP server", - "mcpServer.quickAdd.dialogDescription": "Pick a commonly used MCP server to pre-fill the form below.", + "mcpServer.quickAdd.dialogDescription": "Pick a commonly used MCP server to connect it right away.", "mcpServer.quickAdd.radioGroupLabel": "Available servers", "mcpServer.quickAdd.footerText": "Explore more options in the server catalog.", "mcpServer.quickAdd.cancel": "Cancel", "mcpServer.quickAdd.continue": "Continue", + "mcpServer.quickAdd.connecting": "Connecting…", "mcpServer.quickAdd.emptyState": "No quick add servers available right now.", "mcpServer.quickAdd.errorState": "Unable to load quick add servers. Try again.", + "mcpServer.quickAdd.connectError": "Unable to connect this server. Try again.", + "mcpServer.quickAdd.alreadyConnected": "{name} is already connected. Manage it from the server catalog.", "mcpServer.table.caption": "List of MCP servers with status and actions", "mcpServer.table.name": "Name", "mcpServer.table.components": "Components", diff --git a/src/i18n/locales/es-ES/mcpServer.json b/src/i18n/locales/es-ES/mcpServer.json index 1ca2a95d..1ed0ade3 100644 --- a/src/i18n/locales/es-ES/mcpServer.json +++ b/src/i18n/locales/es-ES/mcpServer.json @@ -32,13 +32,16 @@ "mcpServer.form.saveChanges": "Guardar cambios", "mcpServer.form.connectServer": "Conectar servidor", "mcpServer.quickAdd.dialogTitle": "Conectar servidor MCP", - "mcpServer.quickAdd.dialogDescription": "Elige un servidor MCP de uso frecuente para completar el formulario a continuación.", + "mcpServer.quickAdd.dialogDescription": "Elige un servidor MCP de uso frecuente para conectarlo de inmediato.", "mcpServer.quickAdd.radioGroupLabel": "Servidores disponibles", "mcpServer.quickAdd.footerText": "Explora más opciones en el catálogo de servidores.", "mcpServer.quickAdd.cancel": "Cancelar", "mcpServer.quickAdd.continue": "Continuar", + "mcpServer.quickAdd.connecting": "Conectando…", "mcpServer.quickAdd.emptyState": "No hay servidores de agregado rápido disponibles en este momento.", "mcpServer.quickAdd.errorState": "No se pudieron cargar los servidores de agregado rápido. Vuelve a intentarlo.", + "mcpServer.quickAdd.connectError": "No se pudo conectar este servidor. Inténtalo de nuevo.", + "mcpServer.quickAdd.alreadyConnected": "{name} ya está conectado. Gestiónalo desde el catálogo de servidores.", "mcpServer.table.caption": "Lista de servidores MCP con estado y acciones", "mcpServer.table.name": "Nombre", "mcpServer.table.components": "Componentes", diff --git a/src/i18n/locales/pt-BR/mcpServer.json b/src/i18n/locales/pt-BR/mcpServer.json index e0687e77..b709f1e2 100644 --- a/src/i18n/locales/pt-BR/mcpServer.json +++ b/src/i18n/locales/pt-BR/mcpServer.json @@ -32,13 +32,16 @@ "mcpServer.form.saveChanges": "Salvar alterações", "mcpServer.form.connectServer": "Conectar servidor", "mcpServer.quickAdd.dialogTitle": "Conectar servidor MCP", - "mcpServer.quickAdd.dialogDescription": "Escolha um servidor MCP de uso comum para preencher o formulário abaixo.", + "mcpServer.quickAdd.dialogDescription": "Escolha um servidor MCP de uso comum para conectá-lo imediatamente.", "mcpServer.quickAdd.radioGroupLabel": "Servidores disponíveis", "mcpServer.quickAdd.footerText": "Explore mais opções no catálogo de servidores.", "mcpServer.quickAdd.cancel": "Cancelar", "mcpServer.quickAdd.continue": "Continuar", + "mcpServer.quickAdd.connecting": "Conectando…", "mcpServer.quickAdd.emptyState": "Nenhum servidor de adição rápida disponível no momento.", "mcpServer.quickAdd.errorState": "Não foi possível carregar os servidores de adição rápida. Tente novamente.", + "mcpServer.quickAdd.connectError": "Não foi possível conectar este servidor. Tente novamente.", + "mcpServer.quickAdd.alreadyConnected": "{name} já está conectado. Gerencie-o no catálogo de servidores.", "mcpServer.table.caption": "Lista de servidores MCP com status e ações", "mcpServer.table.name": "Nome", "mcpServer.table.components": "Componentes", From 7f92b685a0b0c4399255b71164f979975bf99d38 Mon Sep 17 00:00:00 2001 From: Anna Effort Date: Mon, 31 Aug 2026 18:48:10 -0700 Subject: [PATCH 2/7] fix: reserve the loaded grid height in the Quick Add dialog DialogContent is vertically centred, so swapping a 16px inline loader for the ~200px card grid re-centred the whole box in one frame and read as a bounce on open. Loading, error and empty now render over a skeleton grid sized from the curated id list. Signed-off-by: Anna Effort --- .../mcp-servers/QuickAddServerDialog.test.tsx | 38 +++++++++++ .../mcp-servers/QuickAddServerDialog.tsx | 63 ++++++++++++++++--- 2 files changed, 91 insertions(+), 10 deletions(-) diff --git a/src/components/mcp-servers/QuickAddServerDialog.test.tsx b/src/components/mcp-servers/QuickAddServerDialog.test.tsx index de176814..899846bd 100644 --- a/src/components/mcp-servers/QuickAddServerDialog.test.tsx +++ b/src/components/mcp-servers/QuickAddServerDialog.test.tsx @@ -295,6 +295,44 @@ describe("QuickAddServerDialog", () => { expect(onBrowseCatalog).toHaveBeenCalled(); }); + // The dialog is vertically centred, so a shorter loading state would re-centre the box the + // moment the grid arrives. Loading, error and empty all reserve the loaded grid's height. + it("reserves the loaded grid's height while the catalog is loading", () => { + mockCatalogQuery({ data: undefined, isLoading: true }); + renderWithProviders( + , + ); + + expect(screen.getByRole("status")).toBeInTheDocument(); + expect(document.querySelectorAll("[aria-hidden='true'] > div.rounded-xl")).toHaveLength( + QUICK_ADD_CATALOG_IDS.length, + ); + }); + + it("keeps the reserved height behind the empty state", () => { + mockCatalogQuery({ + data: { servers: [], total: 0, categories: [], auth_types: [], providers: [] }, + }); + renderWithProviders( + , + ); + + expect(screen.getByText("No quick add servers available right now.")).toBeInTheDocument(); + expect(document.querySelectorAll("[aria-hidden='true'] > div.rounded-xl")).toHaveLength( + QUICK_ADD_CATALOG_IDS.length, + ); + }); + it("shows an error state when the catalog fails to load", () => { mockCatalogQuery({ data: undefined, error: { message: "network error" } }); renderWithProviders( diff --git a/src/components/mcp-servers/QuickAddServerDialog.tsx b/src/components/mcp-servers/QuickAddServerDialog.tsx index 809bf7a0..7d277795 100644 --- a/src/components/mcp-servers/QuickAddServerDialog.tsx +++ b/src/components/mcp-servers/QuickAddServerDialog.tsx @@ -15,13 +15,14 @@ import { } from "@/components/ui/dialog"; import { InlineNotification } from "@/components/ui/inline-notification"; import { Label } from "@/components/ui/label"; -import { Loading } from "@/components/ui/loading"; import { RadioGroup, RadioGroupItem } from "@/components/ui/radio-group"; import { QUICK_ADD_CATALOG_IDS } from "@/config/quickAddServers"; import type { CatalogListResponse, CatalogServer } from "@/generated/types"; import { useQuery } from "@/hooks/useQuery"; +import { cn } from "@/lib/utils"; const CATALOG_PATH = "/v1/catalog?limit=1000"; +const GRID_CLASS = "grid grid-cols-2 gap-3 sm:grid-cols-4"; // Quick Add registers without collecting any credentials, so it can't complete an // OAuth setup flow, and the gateway only supports these two transports. const OPEN_AUTH_TYPE = "Open"; @@ -35,6 +36,37 @@ function isQuickAddEligible(server: CatalogServer | undefined): server is Catalo ); } +/** + * DialogContent is vertically centred with a content-driven height, so anything that changes + * height after the dialog opens re-centres the whole box and reads as a bounce. Every state + * that precedes the loaded grid renders through here, reserving that grid's height: a card per + * curated id, matching the real card's padding, logo box and two-line description. Children, + * when given, are centred over the reserved space instead of the skeleton. + */ +function ReservedGridHeight({ children }: { children?: ReactNode }) { + return ( +
+