diff --git a/AGENTS.md b/AGENTS.md index 40901636..546b7af6 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -106,7 +106,9 @@ Use these patterns when extending list/detail or run-submission UX: 2. **Prefer in-place creation over navigation breaks.** - For `/runs/new`, create profiles in a dialog and keep users on the page. - - Reuse shared forms (e.g. `ProfileCreateForm`) between full-page and modal flows to avoid behavior drift. + - Reuse shared forms (e.g. `ProfileCreateForm`, `McpServerForm`) between full-page and modal flows to avoid behavior drift. + - Keep entity sections visible when their list is empty: show an empty state plus the inline `New…` action instead of hiding the section. + - Inline-create dialogs mounted inside a page `
` (e.g. Submit Run) must call `e.stopPropagation()` in their own submit handler — React bubbles synthetic events through portals to the outer form — and mark their primary button with `data-command-enter` so page-level Cmd+Enter shortcuts don't fire underneath. 3. **Treat action counts as source-of-truth UX.** - Any submit/CTA label must reflect the real backend effect (e.g. expanded run count, not just occurrence count). diff --git a/apps/portal/src/components/McpServerCreateDialog.stories.tsx b/apps/portal/src/components/McpServerCreateDialog.stories.tsx new file mode 100644 index 00000000..1f982a5a --- /dev/null +++ b/apps/portal/src/components/McpServerCreateDialog.stories.tsx @@ -0,0 +1,103 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import { useEffect, useState } from "react"; +import type { Meta, StoryObj } from "@storybook/react-vite"; +import { expect, userEvent, within } from "storybook/test"; +import { http, HttpResponse } from "msw"; +import type { McpServerDocument } from "@/types"; +import { getSelectedProjectId, setSelectedProjectIdHolder } from "@/lib/project-scope"; +import { Button } from "@/components/ui/button"; +import { McpServerCreateDialog } from "./McpServerCreateDialog"; + +const existingServers: McpServerDocument[] = [ + { + _id: "learn-docs", + name: "Learn Docs", + type: "http", + url: "https://learn.example.com/mcp", + createdAt: "2026-01-01T00:00:00.000Z", + }, +]; + +function DialogHarness() { + const [open, setOpen] = useState(true); + const [created, setCreated] = useState(""); + const [scopeReady, setScopeReady] = useState(false); + + useEffect(() => { + const previousProjectId = getSelectedProjectId(); + setSelectedProjectIdHolder("demo-project"); + setScopeReady(true); + return () => setSelectedProjectIdHolder(previousProjectId); + }, []); + + if (!scopeReady) return null; + + return ( +
+ +

{created}

+ { + setCreated(server._id); + setOpen(false); + }} + /> +
+ ); +} + +const meta = { + component: McpServerCreateDialog, + render: () => , + args: { + open: true, + onOpenChange: () => {}, + onCreated: () => {}, + }, + tags: ["ai-generated", "needs-work"], + parameters: { + msw: { + handlers: [ + http.get("*/api/v1/mcp/servers", () => HttpResponse.json(existingServers)), + http.post("*/api/v1/mcp/servers", async ({ request }) => { + const body = (await request.json()) as Partial; + return HttpResponse.json( + { ...body, createdAt: new Date().toISOString() }, + { status: 201 }, + ); + }), + ], + }, + }, +} satisfies Meta; + +export default meta; +type Story = StoryObj; + +export const Default: Story = {}; + +export const DuplicateSlug: Story = { + play: async ({ canvasElement }) => { + const page = within(canvasElement.ownerDocument.body); + const slug = await page.findByLabelText(/slug/i); + await userEvent.type(slug, "learn-docs"); + await expect(await page.findByText(/already exists in this project/i)).toBeVisible(); + await expect(page.getByRole("button", { name: /create server/i })).toBeDisabled(); + }, +}; + +export const CreateAndSelect: Story = { + play: async ({ canvas, canvasElement }) => { + const page = within(canvasElement.ownerDocument.body); + await userEvent.type(await page.findByLabelText(/slug/i), "github-search"); + await userEvent.type(page.getByLabelText(/^url/i), "https://example.com/mcp"); + await userEvent.click(page.getByRole("button", { name: /create server/i })); + await expect(await canvas.findByText("github-search")).toBeVisible(); + }, +}; diff --git a/apps/portal/src/components/McpServerCreateDialog.tsx b/apps/portal/src/components/McpServerCreateDialog.tsx new file mode 100644 index 00000000..cf5e902b --- /dev/null +++ b/apps/portal/src/components/McpServerCreateDialog.tsx @@ -0,0 +1,37 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import { + Dialog, DialogContent, DialogDescription, DialogHeader, DialogTitle, +} from "@/components/ui/dialog"; +import { McpServerForm } from "@/components/McpServerForm"; +import type { McpServerDocument } from "@/types"; + +interface McpServerCreateDialogProps { + open: boolean; + onOpenChange: (open: boolean) => void; + onCreated: (server: McpServerDocument) => void; +} + +export function McpServerCreateDialog({ open, onOpenChange, onCreated }: McpServerCreateDialogProps) { + return ( + + + + New MCP Server + + Register an MCP server without leaving this flow. + + +
+ +
+
+
+ ); +} diff --git a/apps/portal/src/components/McpServerForm.test.tsx b/apps/portal/src/components/McpServerForm.test.tsx new file mode 100644 index 00000000..4e531037 --- /dev/null +++ b/apps/portal/src/components/McpServerForm.test.tsx @@ -0,0 +1,127 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +// @vitest-environment happy-dom +import { afterEach, describe, expect, it, vi } from "vitest"; +import { createPortal } from "react-dom"; +import { cleanup, fireEvent, render, screen, waitFor } from "@testing-library/react"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import type { McpServerDocument } from "@/types"; + +const seed: McpServerDocument[] = [ + { + _id: "learn-docs", + name: "Learn Docs", + type: "http", + url: "https://learn.example.com/mcp", + createdAt: "2026-01-01T00:00:00.000Z", + }, +]; +let servers: McpServerDocument[] = [...seed]; + +vi.mock("@/lib/api", () => ({ + api: { + listMcpServers: vi.fn(async () => servers), + createMcpServer: vi.fn(async (body: { _id: string; name: string; type: McpServerDocument["type"]; url?: string }) => { + const created: McpServerDocument = { ...body, createdAt: "2026-01-02T00:00:00.000Z" }; + servers = [...servers, created]; + return created; + }), + }, +})); + +vi.mock("sonner", () => ({ toast: { success: vi.fn(), error: vi.fn() } })); + +import { api } from "@/lib/api"; +import { McpServerForm } from "./McpServerForm"; +import { McpServerCreateDialog } from "./McpServerCreateDialog"; + +afterEach(() => { + cleanup(); + vi.clearAllMocks(); + servers = [...seed]; +}); + +function renderWithClient( + ui: React.ReactElement, + queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } }), +) { + return render({ui}); +} + +async function fillValidHttpServer(slug: string) { + // Wait for the existing-server list to load so duplicate detection is live. + await waitFor(() => expect(api.listMcpServers).toHaveBeenCalled()); + fireEvent.change(screen.getByLabelText(/slug/i), { target: { value: slug } }); + fireEvent.change(screen.getByLabelText(/^url/i), { target: { value: "https://example.com/mcp" } }); +} + +describe("McpServerForm", () => { + it("blocks a slug that already exists in the project", async () => { + renderWithClient(); + await fillValidHttpServer("learn-docs"); + + expect(await screen.findByText(/already exists in this project/i)).toBeTruthy(); + expect((screen.getByRole("button", { name: /create server/i }) as HTMLButtonElement).disabled).toBe(true); + }); + + it("creates the server, seeds the shared list, and reports it upward", async () => { + const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + // Capture the cache at callback time: callers rely on the new server already + // being in the list when they auto-select it. + let cachedAtCallback: string[] = []; + const onCreated = vi.fn(() => { + cachedAtCallback = (queryClient.getQueryData(["mcp-servers"]) ?? []).map((s) => s._id); + }); + renderWithClient(, queryClient); + await fillValidHttpServer("new-server"); + + const submit = screen.getByRole("button", { name: /create server/i }) as HTMLButtonElement; + await waitFor(() => expect(submit.disabled).toBe(false)); + fireEvent.submit(submit.closest("form")!); + + await waitFor(() => expect(onCreated).toHaveBeenCalledTimes(1)); + expect(vi.mocked(api.createMcpServer).mock.calls[0][0]).toMatchObject({ + _id: "new-server", + name: "New Server", + type: "http", + url: "https://example.com/mcp", + }); + expect(onCreated.mock.calls[0]).toEqual([expect.objectContaining({ _id: "new-server" })]); + expect(cachedAtCallback).toEqual(["new-server", "learn-docs"]); + }); + + it("does not propagate submit to an enclosing page form across a portal", async () => { + const outerSubmit = vi.fn((e: React.FormEvent) => e.preventDefault()); + renderWithClient( + + {createPortal(, document.body)} + , + ); + await fillValidHttpServer("portal-server"); + + const submit = screen.getByRole("button", { name: /create server/i }); + fireEvent.submit(submit.closest("form")!); + + await waitFor(() => expect(api.createMcpServer).toHaveBeenCalledTimes(1)); + expect(outerSubmit).not.toHaveBeenCalled(); + }); +}); + +describe("McpServerCreateDialog", () => { + it("claims Cmd/Ctrl+Enter so page-level shortcuts do not fire underneath", async () => { + const pageShortcut = vi.fn(); + const listener = (e: KeyboardEvent) => { + if (!e.defaultPrevented && e.key === "Enter") pageShortcut(); + }; + document.addEventListener("keydown", listener); + try { + renderWithClient(); + const slug = await screen.findByLabelText(/slug/i); + fireEvent.keyDown(slug, { key: "Enter", metaKey: true, ctrlKey: true }); + expect(pageShortcut).not.toHaveBeenCalled(); + } finally { + document.removeEventListener("keydown", listener); + } + }); +}); diff --git a/apps/portal/src/components/McpServerForm.tsx b/apps/portal/src/components/McpServerForm.tsx new file mode 100644 index 00000000..9eff62a4 --- /dev/null +++ b/apps/portal/src/components/McpServerForm.tsx @@ -0,0 +1,433 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import { useState } from "react"; +import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; +import { api } from "@/lib/api"; +import type { + McpServerDocument, + McpTransportType, + McpServerHeader, + McpSessionMode, +} from "@/types"; +import { Button } from "@/components/ui/button"; +import { Input } from "@/components/ui/input"; +import { Label } from "@/components/ui/label"; +import { Textarea } from "@/components/ui/textarea"; +import { Card, CardContent, CardHeader, CardTitle, CardDescription } from "@/components/ui/card"; +import { + Select, SelectContent, SelectItem, SelectTrigger, SelectValue, +} from "@/components/ui/select"; +import { Plus, Trash2, Loader2, ArrowLeft } from "lucide-react"; +import { toast } from "sonner"; +import { SecretInput } from "@/components/ui/secret-input"; + +const SLUG_REGEX = /^[a-z0-9]([a-z0-9-]*[a-z0-9])?$/; + +/** Convert a display name to a kebab-case slug */ +function nameToSlug(text: string): string { + return text + .toLowerCase() + .replace(/['']/g, "") + .replace(/[^a-z0-9]+/g, "-") + .replace(/^-+|-+$/g, "") + .slice(0, 60); +} + +/** Convert a kebab-case slug to a Title Case display name */ +function slugToName(slug: string): string { + return slug + .split("-") + .map((w) => w.charAt(0).toUpperCase() + w.slice(1)) + .join(" "); +} + +interface McpServerFormProps { + onCreated: (server: McpServerDocument) => void; + onCancel?: () => void; + className?: string; + showCancel?: boolean; + stickyFooter?: boolean; +} + +export function McpServerForm({ + onCreated, + onCancel, + className, + showCancel = true, + stickyFooter = false, +}: McpServerFormProps) { + const queryClient = useQueryClient(); + + // Load the active project's servers so we can flag a duplicate slug before submit. + // (The list is project-scoped; the API is the source of truth and also 409s.) + const { data: existingServers = [] } = useQuery({ + queryKey: ["mcp-servers"], + queryFn: () => api.listMcpServers(), + }); + + const [slug, setSlug] = useState(""); + const [name, setName] = useState(""); + const [slugManuallyEdited, setSlugManuallyEdited] = useState(false); + const [nameManuallyEdited, setNameManuallyEdited] = useState(false); + const [type, setType] = useState("http"); + const [url, setUrl] = useState(""); + const [command, setCommand] = useState(""); + const [args, setArgs] = useState(""); + const [envPairs, setEnvPairs] = useState([]); + const [sessionMode, setSessionMode] = useState("stateless"); + const [version, setVersion] = useState(""); + const [description, setDescription] = useState(""); + const [headers, setHeaders] = useState([]); + + const isStdio = type === "stdio"; + + const handleNameChange = (value: string) => { + setName(value); + setNameManuallyEdited(true); + if (!slugManuallyEdited) { + setSlug(nameToSlug(value)); + } + }; + + const handleSlugChange = (value: string) => { + const lower = value.toLowerCase(); + setSlug(lower); + setSlugManuallyEdited(true); + if (!nameManuallyEdited) { + setName(slugToName(lower)); + } + }; + + const createMutation = useMutation({ + mutationFn: api.createMcpServer, + onSuccess: (data) => { + toast.success(`MCP server "${data.name}" created`); + // Seed the shared list so pickers can select the new server immediately, + // then refetch to pick up any server-side normalization. + queryClient.setQueryData(["mcp-servers"], (previous) => { + const existing = previous ?? []; + if (existing.some((item) => item._id === data._id)) { + return existing.map((item) => (item._id === data._id ? data : item)); + } + return [data, ...existing]; + }); + void queryClient.invalidateQueries({ queryKey: ["mcp-servers"] }); + onCreated(data); + }, + onError: (error) => { + toast.error(error instanceof Error ? error.message : "Failed to create MCP server"); + }, + }); + + // A server's slug must be unique within the project. The list only holds active + // servers, so this catches active collisions instantly; soft-deleted collisions are + // caught by the API's 409 (surfaced via the mutation's onError toast). + const slugExists = existingServers.some((s) => s._id === slug); + const isValid = slug && SLUG_REGEX.test(slug) && !slugExists && name && (isStdio ? !!command : !!url); + + const handleSubmit = (e: React.FormEvent) => { + e.preventDefault(); + // React propagates synthetic events through portals, so without this a submit + // from the inline dialog would also fire an enclosing page form (e.g. Submit Run). + e.stopPropagation(); + if (!isValid) return; + + if (isStdio) { + createMutation.mutate({ + _id: slug, + name, + type, + command, + args: args.trim() ? args.trim().split(/\s+/) : undefined, + env: envPairs.length > 0 ? Object.fromEntries(envPairs.filter(p => p.name && p.value).map(p => [p.name, p.value])) : undefined, + sessionMode, + version: version.trim() || undefined, + ...(description ? { description } : {}), + }); + } else { + createMutation.mutate({ + _id: slug, + name, + type, + url, + ...(description ? { description } : {}), + ...(headers.length > 0 ? { headers: headers.filter(h => h.name && h.value) } : {}), + }); + } + }; + + const addHeader = () => { + setHeaders([...headers, { name: "", value: "" }]); + }; + + const updateHeader = (index: number, field: "name" | "value", val: string) => { + const updated = [...headers]; + updated[index] = { ...updated[index], [field]: val }; + setHeaders(updated); + }; + + const removeHeader = (index: number) => { + setHeaders(headers.filter((_, i) => i !== index)); + }; + + const addEnvPair = () => setEnvPairs([...envPairs, { name: "", value: "" }]); + const updateEnvPair = (index: number, field: "name" | "value", val: string) => { + const updated = [...envPairs]; + updated[index] = { ...updated[index], [field]: val }; + setEnvPairs(updated); + }; + const removeEnvPair = (index: number) => setEnvPairs(envPairs.filter((_, i) => i !== index)); + + return ( +
+ + + Server Details + Configure the remote MCP server connection + + +
+
+ + handleSlugChange(e.target.value)} + pattern="[a-z0-9]([a-z0-9-]*[a-z0-9])?" + className="font-mono" + /> +

+ Lowercase letters, numbers, and hyphens only +

+ {slug && !SLUG_REGEX.test(slug) && ( +

+ Invalid slug format +

+ )} + {slug && SLUG_REGEX.test(slug) && slugExists && ( +

+ An MCP server with this slug already exists in this project. Use the edit flow to change it. +

+ )} +
+
+ + handleNameChange(e.target.value)} + /> +
+
+ +
+
+ + +
+ {!isStdio ? ( +
+ + setUrl(e.target.value)} + className="font-mono text-sm" + /> +
+ ) : ( +
+ + setCommand(e.target.value)} + className="font-mono text-sm" + /> +
+ )} +
+ + {isStdio && ( + <> +
+ + setArgs(e.target.value)} + className="font-mono text-sm" + /> +

Space-separated arguments

+
+
+
+ + setVersion(e.target.value)} + className="font-mono text-sm" + /> +

Pins npm package version

+
+
+ + +
+
+ + )} + +
+ +