diff --git a/e2e/quick-add-server.spec.ts b/e2e/quick-add-server.spec.ts index aa4d5b6..8dadaab 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,41 @@ 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(); + // The dialog owns the scope now, so the register body has to carry it. + expect((await registerRequest).postDataJSON()).toMatchObject({ + visibility: "private", + team_id: null, + }); + // 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 c9cd57f..711a190 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,45 @@ 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.queryByLabelText(/URL/i)).not.toBeInTheDocument(); + }); + + // The user never passes through the connect form, so the scope the components step builds + // the virtual server with has to come from the dialog rather than the form's defaults. + it("carries the visibility chosen in the dialog into the exposed virtual server", async () => { + let exposedBody: Record | undefined; + server.use( + http.post("/api/servers", async ({ request }) => { + exposedBody = (await request.json()) as Record; + return HttpResponse.json({ id: "virtual-server-1" }); + }), ); - expect(screen.getByRole("radio", { name: "Streamable HTTP" })).toBeChecked(); + const user = userEvent.setup(); + renderWithRouter(); + + await user.click(screen.getByRole("button", { name: /mcp server catalog/i })); + await user.click(screen.getByRole("radio", { name: /DeepWiki/i })); + await user.click(screen.getByRole("combobox", { name: "Visibility" })); + await user.click(screen.getByRole("option", { name: "Internal" })); + await user.click(screen.getByRole("button", { name: "Continue" })); + + await user.click(await screen.findByRole("button", { name: "Expose components" })); + + await waitFor(() => expect(exposedBody).toBeDefined()); + expect(exposedBody).toMatchObject({ visibility: "public" }); }); - 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 +1306,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 501002c..116550e 100644 --- a/src/components/mcp-servers/MCPServerForm.tsx +++ b/src/components/mcp-servers/MCPServerForm.tsx @@ -7,22 +7,15 @@ import { Input } from "@/components/ui/input"; import { Textarea } from "@/components/ui/textarea"; import { MCPIcon } from "@/components/icons/MCPIcon"; import { AdvancedSettings } from "@/components/mcp-servers/AdvancedSettings"; -import { QuickAddServerDialog } from "@/components/mcp-servers/QuickAddServerDialog"; +import { + QuickAddServerDialog, + type QuickAddConnection, +} 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"; -} +import type { Visibility } from "@/types/server"; interface MCPServerFormProps { isOpen: boolean; @@ -34,6 +27,9 @@ interface MCPServerFormProps { interface CreatedGatewayInfo { id: string; name: string; + /** Scope for the virtual server built at the components step. Quick Add picks its own. */ + visibility: Visibility; + teamId: string; } export function MCPServerForm({ isOpen, onToggle, serverId, onSuccess }: MCPServerFormProps) { @@ -41,7 +37,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 +105,7 @@ export function MCPServerForm({ isOpen, onToggle, serverId, onSuccess }: MCPServ setQueryParamName, queryParamApiKey, setQueryParamApiKey, - } = useMCPServerForm(serverId, prefill); + } = useMCPServerForm(serverId); const handleRedirectUriChange = useCallback( (uri: string) => { @@ -124,12 +119,15 @@ 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. Its scope comes from the dialog + // rather than the form, which the user never passed through. + const handleQuickAddConnected = useCallback((connection: QuickAddConnection) => { + setCreatedGateway({ + id: connection.gatewayId, + name: connection.serverName, + visibility: connection.visibility, + teamId: connection.teamId, }); setQuickAddOpen(false); }, []); @@ -154,6 +152,8 @@ export function MCPServerForm({ isOpen, onToggle, serverId, onSuccess }: MCPServ const gatewayInfo: CreatedGatewayInfo = { id: gatewayId, name: name, + visibility, + teamId, }; setCreatedGateway(gatewayInfo); } else { @@ -175,8 +175,8 @@ export function MCPServerForm({ isOpen, onToggle, serverId, onSuccess }: MCPServ {chunks} @@ -482,12 +482,15 @@ export function MCPServerForm({ isOpen, onToggle, serverId, onSuccess }: MCPServ - + {/* Mounted only while open: the dialog loads the catalog and the caller's teams. */} + {quickAddOpen && ( + + )} ); } diff --git a/src/components/mcp-servers/QuickAddServerDialog.test.tsx b/src/components/mcp-servers/QuickAddServerDialog.test.tsx index c5d5150..4fba1cd 100644 --- a/src/components/mcp-servers/QuickAddServerDialog.test.tsx +++ b/src/components/mcp-servers/QuickAddServerDialog.test.tsx @@ -1,18 +1,37 @@ -import { describe, expect, it, vi } from "vitest"; -import { screen } from "@testing-library/react"; +import { describe, expect, it, vi, beforeEach } from "vitest"; +import { act, 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"; import { QUICK_ADD_CATALOG_IDS } from "@/config/quickAddServers"; +import type { Team } from "@/types/team"; import { QuickAddServerDialog } from "./QuickAddServerDialog"; vi.mock("@/hooks/useQuery", () => ({ useQuery: vi.fn(), })); +vi.mock("@/api/catalog", () => ({ + registerCatalogServer: vi.fn(), +})); + +const teamScopeState = vi.hoisted(() => ({ + teams: [] as Team[], +})); + +vi.mock("@/hooks/useTeams", () => ({ + useTeamScope: ({ onTeamIdChange }: { onTeamIdChange: (teamId: string) => void }) => ({ + teams: teamScopeState.teams, + onTeamChange: onTeamIdChange, + }), +})); + const mockUseQuery = vi.mocked(useQuery); +const mockRegister = vi.mocked(registerCatalogServer); function catalogServer( overrides: Partial & Pick, @@ -68,14 +87,23 @@ 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(); + teamScopeState.teams = []; + }); + it("renders only the curated catalog entries", () => { mockCatalogQuery(); renderWithProviders( , ); @@ -91,7 +119,7 @@ describe("QuickAddServerDialog", () => { , ); @@ -106,7 +134,7 @@ describe("QuickAddServerDialog", () => { , ); @@ -115,15 +143,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 +164,307 @@ 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], { + visibility: "private", + team_id: null, + }); + await waitFor(() => { + expect(onConnected).toHaveBeenCalledWith({ + gatewayId: "gateway-1", + serverName: QUICK_ADD_CATALOG_IDS[0], + visibility: "private", + teamId: "", + }); + }); + }); + + it("registers once when Continue fires twice before a render commits", async () => { + mockCatalogQuery(); + mockRegister.mockResolvedValue({ + success: true, + server_id: "gateway-1", + message: "registered", + }); + const user = userEvent.setup(); + renderWithProviders( + , + ); + + await selectFirstCuratedCard(user); + + // Dispatched natively and inside one act scope, so neither the isConnecting re-render nor + // the disabled attribute lands between the two clicks. Only the ref guard stops the second. + const continueButton = screen.getByRole("button", { name: "Continue" }); + await act(async () => { + continueButton.click(); + continueButton.click(); + }); + + expect(mockRegister).toHaveBeenCalledTimes(1); + }); + + 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({ + gatewayId: "existing-gateway", + serverName: QUICK_ADD_CATALOG_IDS[0], + visibility: "private", + teamId: "", + }); + }); + + 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("registers with the chosen team and hands that scope to the components step", async () => { + mockCatalogQuery(); + teamScopeState.teams = [ + { id: "team-alpha", name: "Alpha team" }, + { id: "team-beta", name: "Beta team" }, + ] as Team[]; + mockRegister.mockResolvedValue({ + success: true, + server_id: "gateway-1", + message: "registered", + }); + const user = userEvent.setup(); + const onConnected = vi.fn(); + renderWithProviders( + , + ); + + await selectFirstCuratedCard(user); + await user.click(screen.getByRole("combobox", { name: "Visibility" })); + await user.click(screen.getByRole("option", { name: "Team" })); + await user.click(screen.getByRole("combobox", { name: /^Team/ })); + await user.click(screen.getByRole("option", { name: "Alpha team" })); + await user.click(screen.getByRole("button", { name: "Continue" })); + + expect(mockRegister).toHaveBeenCalledWith(QUICK_ADD_CATALOG_IDS[0], { + visibility: "team", + team_id: "team-alpha", + }); + await waitFor(() => { + expect(onConnected).toHaveBeenCalledWith({ + gatewayId: "gateway-1", + serverName: QUICK_ADD_CATALOG_IDS[0], + visibility: "team", + teamId: "team-alpha", + }); + }); + }); + + it("blocks team visibility without a team instead of registering", async () => { + mockCatalogQuery(); + teamScopeState.teams = [ + { id: "team-alpha", name: "Alpha team" }, + { id: "team-beta", name: "Beta team" }, + ] as Team[]; + const user = userEvent.setup(); + renderWithProviders( + , + ); + + await selectFirstCuratedCard(user); + await user.click(screen.getByRole("combobox", { name: "Visibility" })); + await user.click(screen.getByRole("option", { name: "Team" })); + await user.click(screen.getByRole("button", { name: "Continue" })); + + expect(await screen.findByText("Select a team.")).toBeInTheDocument(); + expect(mockRegister).not.toHaveBeenCalled(); + }); + + // Nothing cancels the request, so a dismissal that went through would still land the user on + // the components step once it resolved. + it("refuses to close while a registration is in flight", async () => { + mockCatalogQuery(); + let resolveRegistration: (value: Awaited>) => void; + mockRegister.mockReturnValue( + new Promise((resolve) => { + resolveRegistration = resolve; + }), + ); + const user = userEvent.setup(); + const onOpenChange = vi.fn(); + renderWithProviders( + , + ); + + await selectFirstCuratedCard(user); + await user.click(screen.getByRole("button", { name: "Continue" })); + expect(await screen.findByRole("button", { name: "Connecting…" })).toBeDisabled(); + + await user.click(screen.getByRole("button", { name: "Close" })); + await user.keyboard("{Escape}"); + + expect(onOpenChange).not.toHaveBeenCalled(); + expect(screen.getByRole("button", { name: "Cancel" })).toBeDisabled(); + expect(screen.getByRole("button", { name: "server catalog" })).toBeDisabled(); + + resolveRegistration!({ success: true, server_id: "gateway-1", message: "registered" }); + await waitFor(() => + expect(screen.getByRole("button", { name: "Continue" })).toBeInTheDocument(), + ); + }); + + it("drops a curated entry the backend 404s on so it cannot be retried", async () => { + mockCatalogQuery(); + mockRegister.mockRejectedValue(new ApiError(404, null, "not found")); + 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 no longer available in the catalog.`), + ).toBeInTheDocument(); + expect( + screen.queryByRole("radio", { name: new RegExp(QUICK_ADD_CATALOG_IDS[0]) }), + ).not.toBeInTheDocument(); + expect(screen.getByRole("button", { name: "Continue" })).toBeDisabled(); + expect(onConnected).not.toHaveBeenCalled(); }); it("calls onBrowseCatalog when the browse-catalog link is clicked", async () => { @@ -167,7 +475,7 @@ describe("QuickAddServerDialog", () => { , ); @@ -176,13 +484,93 @@ 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, + ); + }); + + // The grid's height is reserved, so if the visibility field appeared only once the catalog + // resolved, the dialog would still grow by that field and re-centre. + it("renders the visibility field while loading as well as loaded", () => { + mockCatalogQuery({ data: undefined, isLoading: true }); + const { rerender } = renderWithProviders( + , + ); + + expect(screen.getByRole("combobox", { name: "Visibility" })).toBeDisabled(); + + mockCatalogQuery(); + rerender( + , + ); + + expect(screen.getByRole("combobox", { name: "Visibility" })).toBeEnabled(); + }); + + it("hides the visibility field when there is nothing to scope", () => { + mockCatalogQuery({ data: undefined, error: { message: "network error" } }); + renderWithProviders( + , + ); + + expect(screen.queryByRole("combobox", { name: "Visibility" })).not.toBeInTheDocument(); + }); + + 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 801afb3..03c08e3 100644 --- a/src/components/mcp-servers/QuickAddServerDialog.tsx +++ b/src/components/mcp-servers/QuickAddServerDialog.tsx @@ -1,6 +1,10 @@ -import { useEffect, useId, useMemo, useState, type ReactNode } from "react"; +import { useCallback, useEffect, useId, useMemo, useRef, useState, type ReactNode } from "react"; import { useIntl } from "react-intl"; +import { registerCatalogServer } from "@/api/catalog"; +import { ApiError } from "@/api/client"; +import { TeamSelect } from "@/components/common/TeamSelect"; +import { VisibilityInfoPopover } from "@/components/common/VisibilityInfoPopover"; import { MCPIcon } from "@/components/icons/MCPIcon"; import { CatalogLogo } from "@/components/server-catalog/CatalogLogo"; import { Button } from "@/components/ui/button"; @@ -13,15 +17,25 @@ 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 { + Select, + SelectContent, + SelectItem, + SelectTrigger, + SelectValue, +} from "@/components/ui/select"; import { QUICK_ADD_CATALOG_IDS } from "@/config/quickAddServers"; import type { CatalogListResponse, CatalogServer } from "@/generated/types"; import { useQuery } from "@/hooks/useQuery"; +import { useTeamScope } from "@/hooks/useTeams"; +import { cn } from "@/lib/utils"; +import type { Visibility } from "@/types/server"; 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. +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"; const SUPPORTED_TRANSPORTS: ReadonlySet = new Set(["SSE", "STREAMABLEHTTP"]); @@ -33,42 +47,189 @@ 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 ( +
+