From 391605cf78366e1fc9c81cfc3163225ca2051109 Mon Sep 17 00:00:00 2001 From: Kieran Zhang Date: Thu, 1 Oct 2026 01:50:56 +0800 Subject: [PATCH 1/3] feat(settings): add an MCP section for global MCP servers Lists servers from the global mcp.json instantly, probes their state separately, and supports sign-in/out, enable/disable, exposure, add and remove. Changes apply to new sessions. --- apps/desktop/src/dev/mock/scenarios.ts | 8 + apps/desktop/src/pages/settings-mcp.test.tsx | 353 +++++++++ apps/desktop/src/pages/settings-mcp.tsx | 730 ++++++++++++++++++ apps/desktop/src/pages/settings.tsx | 8 + apps/desktop/src/shared/runtime.ts | 197 ++++- .../desktop/src/shared/settings-navigation.ts | 3 +- 6 files changed, 1297 insertions(+), 2 deletions(-) create mode 100644 apps/desktop/src/pages/settings-mcp.test.tsx create mode 100644 apps/desktop/src/pages/settings-mcp.tsx diff --git a/apps/desktop/src/dev/mock/scenarios.ts b/apps/desktop/src/dev/mock/scenarios.ts index 91cc7e55..6136a103 100644 --- a/apps/desktop/src/dev/mock/scenarios.ts +++ b/apps/desktop/src/dev/mock/scenarios.ts @@ -518,6 +518,14 @@ export function createMockApi(): PaceRendererApi { case "search_workspace_files": case "list_provider_auth_status": case "test_provider_connection": + case "get_mcp_config": + case "probe_mcp_servers": + case "set_mcp_server_enabled": + case "set_mcp_server_exposure": + case "login_mcp_server": + case "logout_mcp_server": + case "add_mcp_server": + case "remove_mcp_server": case "update:status": case "get_chat_workspace_root": case "get_runtime_info": diff --git a/apps/desktop/src/pages/settings-mcp.test.tsx b/apps/desktop/src/pages/settings-mcp.test.tsx new file mode 100644 index 00000000..23a05932 --- /dev/null +++ b/apps/desktop/src/pages/settings-mcp.test.tsx @@ -0,0 +1,353 @@ +import { act, render, screen, waitFor, within } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { McpSettingsSection } from "@/pages/settings-mcp"; +import type { PaceRendererApi } from "@/shared/runtime"; +import type { + McpActionResult, + McpConfigReport, + McpProbeReport, +} from "@pace/core"; + +const mcpConfig: McpConfigReport = { + configPath: "/agent/mcp.json", + errors: [], + servers: [ + { + name: "filesystem", + source: "/agent/mcp.json", + enabled: true, + exposure: "codemode", + kind: "stdio", + transport: "npx -y @modelcontextprotocol/server-filesystem .", + usesOAuth: false, + }, + { + name: "sentry", + source: "/agent/mcp.json", + enabled: true, + exposure: "codemode", + kind: "http", + transport: "https://mcp.sentry.dev/mcp", + usesOAuth: true, + }, + { + name: "legacy-db", + source: "/agent/mcp.json", + enabled: true, + exposure: "direct", + kind: "stdio", + transport: "node db-server.js", + usesOAuth: false, + }, + { + name: "playwright", + source: "/agent/mcp.json", + enabled: false, + exposure: "codemode", + kind: "stdio", + transport: "npx -y @playwright/mcp", + usesOAuth: false, + }, + ], +}; + +const mcpProbe: McpProbeReport = { + errors: [], + servers: [ + { + name: "filesystem", + state: "connected", + tools: ["read_file", "write_file", "list_directory"], + }, + { name: "sentry", state: "needs-auth", tools: [] }, + { + name: "legacy-db", + state: "failed", + tools: [], + error: 'MCP server "legacy-db" failed to connect: spawn node ENOENT', + }, + { name: "playwright", state: "disabled", tools: [] }, + ], +}; + +function deferred() { + let resolve!: (value: T) => void; + const promise = new Promise((res) => { + resolve = res; + }); + return { promise, resolve }; +} + +type McpRenderOptions = { + config?: McpConfigReport; + probe?: McpProbeReport; + /** When set, probe_mcp_servers returns this promise instead of resolving. */ + probePromise?: Promise; + handlers?: Record) => unknown>; +}; + +function renderMcpSection(options: McpRenderOptions = {}) { + const invoke = vi.fn(async (command: string, args?: Record) => { + if (command === "get_mcp_config") { + return options.config ?? mcpConfig; + } + + const handler = options.handlers?.[command]; + if (handler) { + return handler(args); + } + + if (command === "probe_mcp_servers") { + return options.probePromise ?? options.probe ?? mcpProbe; + } + + throw new Error(`unexpected backend command ${command}`); + }); + + window.pace = { + invoke: invoke as unknown as PaceRendererApi["invoke"], + onBackendEvent: vi.fn(() => vi.fn()), + onBrowserEvent: vi.fn(() => vi.fn()), + onUpdateEvent: vi.fn(() => vi.fn()), + onWindowFocusChanged: vi.fn(() => vi.fn()), + onNavigateRequest: vi.fn(() => vi.fn()), + }; + + const queryClient = new QueryClient(); + const tree = render( + + + , + ); + + return { + ...tree, + invoke, + countCalls: (command: string) => + invoke.mock.calls.filter(([called]) => called === command).length, + }; +} + +async function findMcpSection() { + return screen.findByRole("region", { name: "MCP" }); +} + +afterEach(() => { + delete window.pace; +}); + +describe("Settings — MCP servers", () => { + it("renders configured rows while the probe is pending, then shows probed states", async () => { + const probe = deferred(); + renderMcpSection({ probePromise: probe.promise }); + + const section = await findMcpSection(); + await within(section).findByText("filesystem"); + for (const name of ["sentry", "legacy-db", "playwright"]) { + expect(within(section).getByText(name)).toBeInTheDocument(); + } + // Enabled rows probe until the result lands; a disabled row knows its state. + expect(within(section).getAllByText("Checking…")).toHaveLength(3); + expect(within(section).getByText("Disabled")).toBeInTheDocument(); + + await act(async () => { + probe.resolve(mcpProbe); + }); + + const sentry = await screen.findByTestId("mcp-server-sentry"); + expect(within(sentry).getByText("Needs sign-in")).toBeInTheDocument(); + expect( + within(sentry).getByRole("button", { name: "Sign in" }), + ).toBeInTheDocument(); + + const filesystem = screen.getByTestId("mcp-server-filesystem"); + expect(within(filesystem).getByText(/3 tools/)).toBeInTheDocument(); + + const legacyDb = screen.getByTestId("mcp-server-legacy-db"); + expect(within(legacyDb).getByText(/ENOENT/)).toBeInTheDocument(); + }); + + it("toggles a server through set_mcp_server_enabled and re-probes", async () => { + const user = userEvent.setup(); + const { invoke, countCalls } = renderMcpSection({ + handlers: { + set_mcp_server_enabled: (args) => ({ + ...mcpConfig, + servers: mcpConfig.servers.map((server) => + server.name === args?.name + ? { ...server, enabled: args.enabled === true } + : server, + ), + }), + }, + }); + + const section = await findMcpSection(); + expect(await within(section).findByText(/3 tools/)).toBeInTheDocument(); + expect(countCalls("probe_mcp_servers")).toBe(1); + + const filesystem = screen.getByTestId("mcp-server-filesystem"); + await user.click( + within(filesystem).getByRole("switch", { name: "Enable filesystem" }), + ); + + await waitFor(() => { + expect(invoke).toHaveBeenCalledWith("set_mcp_server_enabled", { + name: "filesystem", + enabled: false, + }); + }); + await waitFor(() => { + expect(countCalls("probe_mcp_servers")).toBe(2); + }); + }); + + it("shows Checking… while a re-probe is in flight instead of the stale state", async () => { + const user = userEvent.setup(); + const reprobe = deferred(); + let probeCalls = 0; + renderMcpSection({ + handlers: { + probe_mcp_servers: () => { + probeCalls += 1; + return probeCalls === 1 ? mcpProbe : reprobe.promise; + }, + set_mcp_server_enabled: (args) => ({ + ...mcpConfig, + servers: mcpConfig.servers.map((server) => + server.name === args?.name + ? { ...server, enabled: args.enabled === true } + : server, + ), + }), + }, + }); + + const playwright = await screen.findByTestId("mcp-server-playwright"); + expect(await within(playwright).findByText("Disabled")).toBeInTheDocument(); + + await user.click( + within(playwright).getByRole("switch", { name: "Enable playwright" }), + ); + + // The toggle wrote enabled:true and invalidated the probe; while the fresh + // probe is in flight the row must not keep showing the stale "Disabled". + expect(await within(playwright).findByText("Checking…")).toBeInTheDocument(); + expect(within(playwright).queryByText("Disabled")).not.toBeInTheDocument(); + + await act(async () => { + reprobe.resolve({ + errors: [], + servers: [ + ...mcpProbe.servers.filter((server) => server.name !== "playwright"), + { + name: "playwright", + state: "connected", + tools: ["browser_navigate"], + }, + ], + }); + }); + + expect(await within(playwright).findByText("Connected")).toBeInTheDocument(); + }); + + it("shows the pending state during sign-in and reports a failed result", async () => { + const user = userEvent.setup(); + const login = deferred(); + renderMcpSection({ + handlers: { login_mcp_server: () => login.promise }, + }); + + const sentry = await screen.findByTestId("mcp-server-sentry"); + await user.click(within(sentry).getByRole("button", { name: "Sign in" })); + + expect( + await within(sentry).findByRole("button", { name: "Waiting for browser…" }), + ).toBeDisabled(); + + await act(async () => { + login.resolve({ ok: false, message: "Sign-in timed out." }); + }); + + expect(await within(sentry).findByRole("alert")).toHaveTextContent( + "Sign-in timed out.", + ); + }); + + it("keeps the add dialog open on a failed result and splits arguments by line", async () => { + const user = userEvent.setup(); + const add = vi.fn(async () => ({ + ok: false, + message: 'An MCP server named "github" already exists.', + })); + const { invoke } = renderMcpSection({ + handlers: { add_mcp_server: add }, + }); + + const section = await findMcpSection(); + await user.click(within(section).getByRole("button", { name: "Add server" })); + + const dialog = await screen.findByRole("dialog", { name: "Add server" }); + const submit = within(dialog).getByRole("button", { name: "Add server" }); + expect(submit).toBeDisabled(); + + await user.type(within(dialog).getByLabelText("Name"), "github"); + await user.type(within(dialog).getByLabelText("Command"), "npx"); + await user.type( + within(dialog).getByLabelText("Arguments"), + "-y\n@modelcontextprotocol/server-github\n\n--flag value", + ); + expect(submit).toBeEnabled(); + + await user.click(submit); + + await waitFor(() => { + expect(invoke).toHaveBeenCalledWith("add_mcp_server", { + input: { + kind: "stdio", + name: "github", + command: "npx", + args: ["-y", "@modelcontextprotocol/server-github", "--flag value"], + }, + }); + }); + expect( + await within(dialog).findByRole("alert"), + ).toHaveTextContent('already exists'); + expect( + screen.getByRole("dialog", { name: "Add server" }), + ).toBeInTheDocument(); + }); + + it("removes a server only after confirmation", async () => { + const user = userEvent.setup(); + const remove = vi.fn(async () => ({ ok: true, message: "" })); + const { invoke } = renderMcpSection({ + handlers: { remove_mcp_server: remove }, + }); + + const filesystem = await screen.findByTestId("mcp-server-filesystem"); + await user.click( + within(filesystem).getByRole("button", { name: "filesystem actions" }), + ); + await user.click(await screen.findByRole("menuitem", { name: /Remove/ })); + + const dialog = await screen.findByRole("dialog", { + name: "Remove MCP server", + }); + expect(invoke).not.toHaveBeenCalledWith( + "remove_mcp_server", + expect.anything(), + ); + + await user.click(within(dialog).getByRole("button", { name: "Remove" })); + await waitFor(() => { + expect(invoke).toHaveBeenCalledWith("remove_mcp_server", { + name: "filesystem", + }); + }); + }); +}); diff --git a/apps/desktop/src/pages/settings-mcp.tsx b/apps/desktop/src/pages/settings-mcp.tsx new file mode 100644 index 00000000..259a23f8 --- /dev/null +++ b/apps/desktop/src/pages/settings-mcp.tsx @@ -0,0 +1,730 @@ +// Settings → MCP: manage the global Pi mcp.json through the backend's +// `pi mcp` bridge. The config list renders instantly; connection states come +// from a separate probe query that can take as long as a server's timeout. + +import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; +import { Banner } from "@astryxdesign/core/Banner"; +import { Button } from "@astryxdesign/core/Button"; +import { Code } from "@astryxdesign/core/Code"; +import { Collapsible } from "@astryxdesign/core/Collapsible"; +import { Dialog, DialogHeader } from "@astryxdesign/core/Dialog"; +import { EmptyState } from "@astryxdesign/core/EmptyState"; +import { Layout, LayoutContent } from "@astryxdesign/core/Layout"; +import { List, ListItem } from "@astryxdesign/core/List"; +import { MoreMenu } from "@astryxdesign/core/MoreMenu"; +import { Selector, SelectorOption } from "@astryxdesign/core/Selector"; +import { + SegmentedControl, + SegmentedControlItem, +} from "@astryxdesign/core/SegmentedControl"; +import { StatusDot, type StatusDotVariant } from "@astryxdesign/core/StatusDot"; +import { HStack, VStack } from "@astryxdesign/core/Stack"; +import { Switch } from "@astryxdesign/core/Switch"; +import { TextArea } from "@astryxdesign/core/TextArea"; +import { TextInput } from "@astryxdesign/core/TextInput"; +import { Heading, Text } from "@astryxdesign/core/Text"; +import { useState } from "react"; +import { MoreHorizontal, RefreshCw } from "@/shared/ui/icons"; +import { invoke } from "@/shared/runtime"; +import type { + McpActionResult, + McpAddServerInput, + McpConfigReport, + McpExposure, + McpProbeReport, + McpServerConfigItem, + McpServerProbe, +} from "@pace/core"; + +const MCP_CONFIG_KEY = ["mcp-config"] as const; +const MCP_PROBE_KEY = ["mcp-probe"] as const; + +const EXPOSURE_OPTIONS: Array<{ value: McpExposure; description: string }> = [ + { value: "codemode", description: "Called from codemode scripts, listed in its description" }, + { value: "codemode-deferred", description: "Called from codemode scripts, found with searchTools()" }, + { value: "deferred", description: "Loaded by tool_search, then called directly" }, + { value: "direct", description: "Declared to the model like built-in tools" }, + { value: "hidden", description: "Registered but unreachable" }, +]; + +type RowStatus = { + variant: StatusDotVariant; + label: string; + pulsing?: boolean; +}; + +function rowStatus( + server: McpServerConfigItem, + probe: McpServerProbe | undefined, + probeFailed: boolean, +): RowStatus { + if (!server.enabled) { + return { variant: "neutral", label: "Disabled" }; + } + if (probe) { + switch (probe.state) { + case "connected": + return { variant: "success", label: "Connected" }; + case "needs-auth": + return { variant: "warning", label: "Needs sign-in" }; + case "failed": + return { variant: "error", label: "Failed" }; + case "disabled": + return { variant: "neutral", label: "Disabled" }; + case "connecting": + return { variant: "neutral", label: "Connecting…", pulsing: true }; + case "disconnected": + return { variant: "neutral", label: "Disconnected" }; + } + } + if (probeFailed) { + return { variant: "neutral", label: "Status unknown" }; + } + return { variant: "neutral", label: "Checking…", pulsing: true }; +} + +function errorText(error: unknown): string { + return error instanceof Error ? error.message : String(error); +} + +function McpServerTools({ tools }: { tools: string[] }) { + const [open, setOpen] = useState(false); + return ( + + {`${tools.length} tool${tools.length === 1 ? "" : "s"}`} + + } + isOpen={open} + onOpenChange={setOpen} + > + {open ? ( + + {tools.join(", ")} + + ) : null} + + ); +} + +function McpExposureSelect({ + label, + value, + disabled, + isLabelHidden = true, + onChange, +}: { + label: string; + value: McpExposure; + disabled?: boolean; + /** Rows hide the label; the add dialog shows it like the other fields. */ + isLabelHidden?: boolean; + onChange: (value: McpExposure) => void; +}) { + return ( + ({ + value: option.value, + label: option.value, + }))} + value={value} + isDisabled={disabled} + onChange={(next) => onChange(next as McpExposure)} + renderOption={(option) => { + const meta = EXPOSURE_OPTIONS.find((entry) => entry.value === option.value); + return ( + + ); + }} + /> + ); +} + +function McpServerRow({ + server, + probe, + probeFailed, +}: { + server: McpServerConfigItem; + probe?: McpServerProbe; + probeFailed: boolean; +}) { + const queryClient = useQueryClient(); + const [rowError, setRowError] = useState(null); + const [removeOpen, setRemoveOpen] = useState(false); + const [removeError, setRemoveError] = useState(null); + + const applyConfigReport = (report: McpConfigReport) => { + queryClient.setQueryData(MCP_CONFIG_KEY, report); + void queryClient.invalidateQueries({ queryKey: MCP_PROBE_KEY }); + }; + + const enabledMutation = useMutation({ + mutationFn: (enabled: boolean) => + invoke("set_mcp_server_enabled", { + name: server.name, + enabled, + }), + onMutate: () => setRowError(null), + onSuccess: applyConfigReport, + onError: (error) => setRowError(errorText(error)), + }); + + const exposureMutation = useMutation({ + mutationFn: (exposure: McpExposure) => + invoke("set_mcp_server_exposure", { + name: server.name, + exposure, + }), + onMutate: () => setRowError(null), + onSuccess: applyConfigReport, + onError: (error) => setRowError(errorText(error)), + }); + + const signInMutation = useMutation({ + mutationFn: () => invoke("login_mcp_server", { name: server.name }), + onMutate: () => setRowError(null), + onSuccess: (result) => { + if (result.ok) { + void queryClient.invalidateQueries({ queryKey: MCP_PROBE_KEY }); + } else { + setRowError(result.message || "Sign-in failed."); + } + }, + onError: (error) => setRowError(errorText(error)), + }); + + const signOutMutation = useMutation({ + mutationFn: () => invoke("logout_mcp_server", { name: server.name }), + onMutate: () => setRowError(null), + onSuccess: (result) => { + if (result.ok) { + void queryClient.invalidateQueries({ queryKey: MCP_PROBE_KEY }); + } else { + setRowError(result.message || "Sign-out failed."); + } + }, + onError: (error) => setRowError(errorText(error)), + }); + + const removeMutation = useMutation({ + mutationFn: () => invoke("remove_mcp_server", { name: server.name }), + onMutate: () => setRemoveError(null), + onSuccess: (result) => { + if (result.ok) { + setRemoveOpen(false); + void queryClient.invalidateQueries({ queryKey: MCP_CONFIG_KEY }); + void queryClient.invalidateQueries({ queryKey: MCP_PROBE_KEY }); + } else { + setRemoveError(result.message || `Could not remove ${server.name}.`); + } + }, + onError: (error) => setRemoveError(errorText(error)), + }); + + const status = rowStatus(server, probe, probeFailed); + const controlsBusy = enabledMutation.isPending || exposureMutation.isPending; + const oauthBusy = signInMutation.isPending || signOutMutation.isPending; + const showSignIn = server.usesOAuth && probe?.state === "needs-auth"; + const showSignOut = server.usesOAuth && probe?.state === "connected"; + + return ( + <> + + + + {status.label} + + + + {server.transport} + + + {probe?.state === "connected" && probe.tools.length > 0 ? ( + + ) : null} + {probe?.state === "connected" && probe.tools.length === 0 ? ( + No tools + ) : null} + {probe?.state === "failed" && probe.error ? ( + + + {probe.error} + + + ) : null} + {rowError ? ( + + {rowError} + + ) : null} + + } + endContent={ + + {showSignIn ? ( +