diff --git a/clients/web/src/App.test.tsx b/clients/web/src/App.test.tsx index 75faad77..674a106e 100644 --- a/clients/web/src/App.test.tsx +++ b/clients/web/src/App.test.tsx @@ -1,4 +1,4 @@ -import { describe, it, expect, beforeEach, vi } from "vitest"; +import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; import { renderWithMantine, screen, @@ -53,6 +53,8 @@ vi.mock("@inspector/core/mcp/index.js", () => { getOAuthState = vi.fn().mockReturnValue(undefined); getPendingSamples = vi.fn().mockReturnValue([]); getPendingElicitations = vi.fn().mockReturnValue([]); + getRoots = vi.fn().mockReturnValue([]); + setRoots = vi.fn().mockResolvedValue(undefined); } const instances: FakeInspectorClient[] = []; return { @@ -187,7 +189,9 @@ vi.mock("@inspector/core/react/useFetchRequestLog.js", () => ({ })); vi.mock("@inspector/core/react/useSettingsDraft.js", () => ({ useSettingsDraft: vi.fn(() => ({ - draft: undefined, + // `draft` is widened so tests can override the return with a populated + // settings draft via `mockReturnValue` (the roots live-apply-on-close path). + draft: undefined as InspectorServerSettings | undefined, onChange: vi.fn(), flush: vi.fn(), })), @@ -243,6 +247,7 @@ vi.mock("./components/views/InspectorView/InspectorView", () => ({ onCancelTask: (taskId: string) => void; onClearCompletedTasks: () => void; onRefreshTasks: () => void; + onServerSettings: (id: string) => void; }) => (
@@ -339,6 +344,7 @@ vi.mock("./components/views/InspectorView/InspectorView", () => ({ read-resource +
), })); @@ -347,6 +353,8 @@ import App from "./App"; import * as McpIndex from "@inspector/core/mcp/index.js"; import { useManagedRequestorTasks } from "@inspector/core/react/useManagedRequestorTasks.js"; import { useInspectorClient } from "@inspector/core/react/useInspectorClient.js"; +import { useSettingsDraft } from "@inspector/core/react/useSettingsDraft.js"; +import type { InspectorServerSettings } from "@inspector/core/mcp/types.js"; // Default useInspectorClient return — capabilities empty (no task tool calls). // Individual tests override via vi.mocked(...).mockReturnValue(...). @@ -843,3 +851,102 @@ describe("App task wiring", () => { ); }); }); + +// Live-apply roots on settings-dialog close: the App diffs the final draft +// roots against what the connected client advertises and calls `setRoots` +// once (not per keystroke), and only for the active server. App.tsx is +// excluded from the coverage gate, but the acceptance criterion ("editing +// roots on a live connection notifies the server on dialog close") lives +// here, so it's worth a direct test of the gate/diff/notify path. +type RootsFakeClient = EventTarget & { + setRoots: ReturnType; + getRoots: ReturnType; +}; + +const settingsWithRoots = ( + roots: InspectorServerSettings["roots"], +): InspectorServerSettings => ({ + headers: [], + metadata: [], + connectionTimeout: 0, + requestTimeout: 0, + taskTtl: 60000, + roots, +}); + +describe("App roots live-apply on settings-dialog close", () => { + beforeEach(() => { + clientInstances.length = 0; + vi.mocked(useInspectorClient).mockReturnValue(DEFAULT_USE_INSPECTOR_CLIENT); + }); + + afterEach(() => { + // Restore the default empty draft so the override doesn't leak. + vi.mocked(useSettingsDraft).mockReturnValue({ + draft: undefined, + onChange: vi.fn(), + flush: vi.fn(), + }); + }); + + async function openSettingsForConnectedServer( + draft: InspectorServerSettings, + ): Promise { + vi.mocked(useSettingsDraft).mockReturnValue({ + draft, + onChange: vi.fn(), + flush: vi.fn(), + }); + const user = userEvent.setup(); + renderWithMantine(); + await user.click(screen.getByText("connect")); + await waitFor(() => expect(clientInstances).toHaveLength(1)); + await user.click(screen.getByText("open-settings")); + await waitFor(() => + expect(screen.getByText("Server Settings")).toBeInTheDocument(), + ); + return clientInstances[0] as RootsFakeClient; + } + + async function closeModal(user: ReturnType) { + const closeBtn = document.querySelector( + "button.mantine-CloseButton-root", + ) as HTMLButtonElement | null; + expect(closeBtn).not.toBeNull(); + await user.click(closeBtn!); + } + + it("calls setRoots once with cleaned roots when roots changed on the active server", async () => { + const user = userEvent.setup(); + const client = await openSettingsForConnectedServer( + // A blank-uri row (left mid-edit) must be dropped; the named root kept. + settingsWithRoots([{ uri: "file:///x", name: "X" }, { uri: "" }]), + ); + // Client currently advertises no roots → the draft differs → notify. + client.getRoots.mockReturnValue([]); + + await closeModal(user); + + await waitFor(() => expect(client.setRoots).toHaveBeenCalledTimes(1)); + expect(client.setRoots).toHaveBeenCalledWith([ + { uri: "file:///x", name: "X" }, + ]); + }); + + it("does not call setRoots when the cleaned roots match what the client advertises", async () => { + const user = userEvent.setup(); + const client = await openSettingsForConnectedServer( + settingsWithRoots([{ uri: "file:///x" }]), + ); + // Same roots already advertised → no notification on close. + client.getRoots.mockReturnValue([{ uri: "file:///x" }]); + + await closeModal(user); + + // Let any close-handler microtasks settle, then assert no notification. + await waitFor(() => + expect(screen.queryByText("Server Settings")).not.toBeInTheDocument(), + ); + expect(client.setRoots).not.toHaveBeenCalled(); + }); +}); diff --git a/clients/web/src/App.tsx b/clients/web/src/App.tsx index d21e33be..5dc00dab 100644 --- a/clients/web/src/App.tsx +++ b/clients/web/src/App.tsx @@ -42,7 +42,10 @@ import { ManagedResourcesState } from "@inspector/core/mcp/state/managedResource import { ManagedResourceTemplatesState } from "@inspector/core/mcp/state/managedResourceTemplatesState.js"; import { ManagedRequestorTasksState } from "@inspector/core/mcp/state/managedRequestorTasksState.js"; import { ResourceSubscriptionsState } from "@inspector/core/mcp/state/resourceSubscriptionsState.js"; -import { serializeMcpConfig } from "@inspector/core/mcp/serverList.js"; +import { + cleanRoots, + serializeMcpConfig, +} from "@inspector/core/mcp/serverList.js"; import { MessageLogState } from "@inspector/core/mcp/state/messageLogState.js"; import { FetchRequestLogState } from "@inspector/core/mcp/state/fetchRequestLogState.js"; import { StderrLogState } from "@inspector/core/mcp/state/stderrLogState.js"; @@ -197,6 +200,7 @@ const EMPTY_SETTINGS: InspectorServerSettings = { connectionTimeout: 0, requestTimeout: 0, taskTtl: DEFAULT_TASK_TTL_MS, + roots: [], }; // Body of the output-schema-mismatch warning toast: a one-line summary plus a @@ -951,6 +955,11 @@ function App() { // Sampling / elicitation are on by default; keep the parameterized // options off until the UI grows the surface to render them. elicit: { form: true, url: true }, + // Always advertise the roots capability (even with no configured + // roots) so the server can issue roots/list and receive + // roots/list_changed; the configured roots are the answer to + // roots/list. Empty-uri rows are dropped before they reach the wire. + roots: cleanRoots(savedSettings?.roots ?? []), ...(savedSettings && savedSettings.requestTimeout > 0 && { timeout: savedSettings.requestTimeout, @@ -1693,8 +1702,36 @@ function App() { const onSettingsModalClose = useCallback(() => { flushSettingsDraft(); + // Apply root edits to the live client once, on close — not on every + // keystroke. `setRoots` fires `notifications/roots/list_changed`, which + // makes the server re-request `roots/list`; doing that per character while + // the user types a URI would flood the wire. We diff the final draft roots + // against what the client currently advertises (both cleaned) and notify + // only when they actually differ, and only for the connected server. + if ( + inspectorClient && + settingsModalTargetId !== undefined && + settingsModalTargetId === activeServerId && + settingsDraft + ) { + const nextRoots = cleanRoots(settingsDraft.roots); + const currentRoots = cleanRoots(inspectorClient.getRoots()); + if (JSON.stringify(nextRoots) !== JSON.stringify(currentRoots)) { + void inspectorClient.setRoots(nextRoots).catch(() => { + // setRoots swallows notification failures internally; a throw here + // only means the client is mid-teardown — the persisted roots will + // re-advertise on the next connect, so nothing to surface. + }); + } + } setSettingsModalTargetId(undefined); - }, [flushSettingsDraft]); + }, [ + flushSettingsDraft, + inspectorClient, + settingsModalTargetId, + activeServerId, + settingsDraft, + ]); // The Resources screen needs `isSubscribed` to flip the Subscribe button // label to "Unsubscribe". Derive it from the live subscriptions list rather diff --git a/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.stories.tsx b/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.stories.tsx index 0220c062..6c05f733 100644 --- a/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.stories.tsx +++ b/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.stories.tsx @@ -13,6 +13,7 @@ const defaultSettings: InspectorServerSettings = { connectionTimeout: 30000, requestTimeout: 60000, taskTtl: 60000, + roots: [], }; function InteractiveRender(args: ServerSettingsFormProps) { @@ -171,6 +172,10 @@ export const AllConfigured: Story = { oauthClientId: "my-client-id", oauthClientSecret: "super-secret-value", oauthScopes: "read write", + roots: [ + { uri: "file:///home/user/project", name: "Project" }, + { uri: "file:///tmp" }, + ], }, }, }; diff --git a/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.test.tsx b/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.test.tsx index 319b170f..c8ce9bdf 100644 --- a/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.test.tsx +++ b/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.test.tsx @@ -13,6 +13,7 @@ const emptySettings: InspectorServerSettings = { connectionTimeout: 30000, requestTimeout: 60000, taskTtl: 60000, + roots: [], }; const populatedSettings: InspectorServerSettings = { @@ -24,6 +25,7 @@ const populatedSettings: InspectorServerSettings = { oauthClientId: "cid", oauthClientSecret: "secret", oauthScopes: "read", + roots: [{ uri: "file:///project", name: "Project" }], }; const allSections: ServerSettingsSection[] = [ @@ -31,6 +33,7 @@ const allSections: ServerSettingsSection[] = [ "metadata", "timeouts", "oauth", + "roots", ]; const baseHandlers = { @@ -43,6 +46,9 @@ const baseHandlers = { onMetadataChange: vi.fn(), onTimeoutChange: vi.fn(), onOAuthChange: vi.fn(), + onAddRoot: vi.fn(), + onRemoveRoot: vi.fn(), + onRootChange: vi.fn(), }; describe("ServerSettingsForm", () => { @@ -58,6 +64,7 @@ describe("ServerSettingsForm", () => { expect(screen.getByText("Request Metadata")).toBeInTheDocument(); expect(screen.getByText("Timeouts")).toBeInTheDocument(); expect(screen.getByText("OAuth Settings")).toBeInTheDocument(); + expect(screen.getByText("Roots")).toBeInTheDocument(); }); it("shows empty hints for headers and metadata when no entries exist", () => { @@ -288,6 +295,71 @@ describe("ServerSettingsForm", () => { }); }); + it("shows the empty hint for roots when none are configured", () => { + renderWithMantine( + , + ); + expect(screen.getByText("No roots configured")).toBeInTheDocument(); + }); + + it("invokes onAddRoot when + Add Root is clicked", async () => { + const user = userEvent.setup(); + const onAddRoot = vi.fn(); + renderWithMantine( + , + ); + await user.click(screen.getByRole("button", { name: "+ Add Root" })); + expect(onAddRoot).toHaveBeenCalledTimes(1); + }); + + it("renders a row's uri and optional name and reports edits via onRootChange", async () => { + const user = userEvent.setup(); + const onRootChange = vi.fn(); + renderWithMantine( + , + ); + // populatedSettings has one root { uri: "file:///project", name: "Project" } + expect(screen.getByDisplayValue("file:///project")).toBeInTheDocument(); + const nameInput = screen.getByDisplayValue("Project"); + await user.type(nameInput, "X"); + expect(onRootChange).toHaveBeenCalled(); + const lastCall = + onRootChange.mock.calls[onRootChange.mock.calls.length - 1]; + expect(lastCall[0]).toBe(0); + // uri is threaded through unchanged when only the name changes + expect(lastCall[1]).toBe("file:///project"); + }); + + it("invokes onRemoveRoot when X is clicked on a root row", async () => { + const user = userEvent.setup(); + const onRemoveRoot = vi.fn(); + renderWithMantine( + , + ); + const removeButtons = screen.getAllByRole("button", { name: "X" }); + await user.click(removeButtons[0]); + expect(onRemoveRoot).toHaveBeenCalledWith(0); + }); + it("invokes onExpandedSectionsChange when an Accordion section is toggled", async () => { const user = userEvent.setup(); const onExpandedSectionsChange = vi.fn(); diff --git a/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.tsx b/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.tsx index 213dd9af..25479aa0 100644 --- a/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.tsx +++ b/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.tsx @@ -12,12 +12,14 @@ import type { InspectorServerSettings, OAuthSettings, } from "@inspector/core/mcp/types.js"; +import type { Root } from "@modelcontextprotocol/sdk/types.js"; export type ServerSettingsSection = | "headers" | "metadata" | "timeouts" - | "oauth"; + | "oauth" + | "roots"; export interface ServerSettingsFormProps { settings: InspectorServerSettings; @@ -34,6 +36,9 @@ export interface ServerSettingsFormProps { value: number, ) => void; onOAuthChange: (oauth: OAuthSettings) => void; + onAddRoot: () => void; + onRemoveRoot: (index: number) => void; + onRootChange: (index: number, uri: string, name: string) => void; } const RemoveIcon = ActionIcon.withProps({ @@ -91,6 +96,42 @@ function KeyValueRows({ ); } +function RootRows({ + roots, + onChange, + onRemove, +}: { + roots: Root[]; + onChange: (index: number, uri: string, name: string) => void; + onRemove: (index: number) => void; +}) { + if (roots.length === 0) { + return null; + } + + return ( + <> + {roots.map((root, index) => ( + + + onChange(index, e.currentTarget.value, root.name ?? "") + } + /> + onChange(index, root.uri, e.currentTarget.value)} + /> + onRemove(index)}>X + + ))} + + ); +} + export function ServerSettingsForm({ settings, expandedSections, @@ -103,6 +144,9 @@ export function ServerSettingsForm({ onMetadataChange, onTimeoutChange, onOAuthChange, + onAddRoot, + onRemoveRoot, + onRootChange, }: ServerSettingsFormProps) { const handleTimeoutChange = (field: "connectionTimeout" | "requestTimeout" | "taskTtl") => @@ -204,6 +248,30 @@ export function ServerSettingsForm({ + + Roots + + + + + Configure the root directories that the server can access. Each + root needs a URI; the name is optional. + + + Add Root + + {settings.roots.length === 0 ? ( + No roots configured + ) : ( + + )} + + + + OAuth Settings diff --git a/clients/web/src/components/groups/ServerSettingsModal/ServerSettingsModal.stories.tsx b/clients/web/src/components/groups/ServerSettingsModal/ServerSettingsModal.stories.tsx index c4a1b167..16d56efb 100644 --- a/clients/web/src/components/groups/ServerSettingsModal/ServerSettingsModal.stories.tsx +++ b/clients/web/src/components/groups/ServerSettingsModal/ServerSettingsModal.stories.tsx @@ -20,6 +20,10 @@ const initialSettings: InspectorServerSettings = { oauthClientId: "my-client-id", oauthClientSecret: "super-secret-value", oauthScopes: "read write", + roots: [ + { uri: "file:///home/user/project", name: "Project" }, + { uri: "file:///tmp" }, + ], }; function InteractiveRender(args: ServerSettingsModalProps) { @@ -69,6 +73,7 @@ export const EmptySettings: Story = { connectionTimeout: 30000, requestTimeout: 60000, taskTtl: 60000, + roots: [], }, }, }; diff --git a/clients/web/src/components/groups/ServerSettingsModal/ServerSettingsModal.test.tsx b/clients/web/src/components/groups/ServerSettingsModal/ServerSettingsModal.test.tsx index 3605249d..7709a563 100644 --- a/clients/web/src/components/groups/ServerSettingsModal/ServerSettingsModal.test.tsx +++ b/clients/web/src/components/groups/ServerSettingsModal/ServerSettingsModal.test.tsx @@ -13,6 +13,7 @@ const initialSettings: InspectorServerSettings = { oauthClientId: "cid", oauthClientSecret: "secret", oauthScopes: "read", + roots: [{ uri: "file:///project", name: "Project" }], }; const emptySettings: InspectorServerSettings = { @@ -21,6 +22,7 @@ const emptySettings: InspectorServerSettings = { connectionTimeout: 30000, requestTimeout: 60000, taskTtl: 60000, + roots: [], }; describe("ServerSettingsModal", () => { @@ -225,6 +227,67 @@ describe("ServerSettingsModal", () => { expect(call.oauthClientId).toBe("a"); }); + it("calls onSettingsChange with a blank row when adding a root", async () => { + const user = userEvent.setup(); + const onSettingsChange = vi.fn(); + renderWithMantine( + , + ); + await user.click(screen.getByRole("button", { name: "Roots" })); + await user.click(screen.getByRole("button", { name: "+ Add Root" })); + expect(onSettingsChange).toHaveBeenCalledWith({ + ...emptySettings, + roots: [{ uri: "", name: "" }], + }); + }); + + it("calls onSettingsChange when removing a root", async () => { + const user = userEvent.setup(); + const onSettingsChange = vi.fn(); + renderWithMantine( + , + ); + await user.click(screen.getByRole("button", { name: "Roots" })); + const removeButtons = screen.getAllByRole("button", { name: "X" }); + // The roots X is the last one (headers section is also expanded). + await user.click(removeButtons[removeButtons.length - 1]); + expect(onSettingsChange).toHaveBeenCalledWith({ + ...initialSettings, + roots: [], + }); + }); + + it("calls onSettingsChange when editing a root uri/name", async () => { + const user = userEvent.setup(); + const onSettingsChange = vi.fn(); + renderWithMantine( + , + ); + await user.click(screen.getByRole("button", { name: "Roots" })); + const uriInput = screen.getByDisplayValue("file:///project"); + await user.type(uriInput, "/src"); + expect(onSettingsChange).toHaveBeenCalled(); + const lastCall = + onSettingsChange.mock.calls[onSettingsChange.mock.calls.length - 1][0]; + expect(lastCall.roots[0].name).toBe("Project"); + expect(lastCall.roots[0].uri).toContain("file:///project"); + }); + it("toggles all accordion sections when ListToggle button is clicked", async () => { const user = userEvent.setup(); renderWithMantine( @@ -243,6 +306,7 @@ describe("ServerSettingsModal", () => { }); const timeoutsControl = screen.getByRole("button", { name: "Timeouts" }); const oauthControl = screen.getByRole("button", { name: "OAuth Settings" }); + const rootsControl = screen.getByRole("button", { name: "Roots" }); // Initially only "headers" is expanded. expect(headersControl.getAttribute("aria-expanded")).toBe("true"); @@ -255,6 +319,7 @@ describe("ServerSettingsModal", () => { expect(metadataControl.getAttribute("aria-expanded")).toBe("true"); expect(timeoutsControl.getAttribute("aria-expanded")).toBe("true"); expect(oauthControl.getAttribute("aria-expanded")).toBe("true"); + expect(rootsControl.getAttribute("aria-expanded")).toBe("true"); // After expanding every section the toggle flips to "Collapse all". await user.click(screen.getByRole("button", { name: "Collapse all" })); diff --git a/clients/web/src/components/groups/ServerSettingsModal/ServerSettingsModal.tsx b/clients/web/src/components/groups/ServerSettingsModal/ServerSettingsModal.tsx index a25e6f5b..9e67461d 100644 --- a/clients/web/src/components/groups/ServerSettingsModal/ServerSettingsModal.tsx +++ b/clients/web/src/components/groups/ServerSettingsModal/ServerSettingsModal.tsx @@ -15,6 +15,7 @@ const ALL_SECTIONS: ServerSettingsSection[] = [ "metadata", "timeouts", "oauth", + "roots", ]; export interface ServerSettingsModalProps { @@ -98,6 +99,29 @@ export function ServerSettingsModal({ }); } + function handleAddRoot() { + onSettingsChange({ + ...settings, + roots: [...settings.roots, { uri: "", name: "" }], + }); + } + + function handleRemoveRoot(index: number) { + onSettingsChange({ + ...settings, + roots: settings.roots.filter((_, i) => i !== index), + }); + } + + function handleRootChange(index: number, uri: string, name: string) { + // Spread the existing root so any non-form fields (e.g. `_meta` from a + // hand-edited mcp.json) survive an edit; only `uri`/`name` are overwritten. + const roots = settings.roots.map((r, i) => + i === index ? { ...r, uri, name } : r, + ); + onSettingsChange({ ...settings, roots }); + } + return ( diff --git a/clients/web/src/test/core/mcp/remote/remoteClientTransport.test.ts b/clients/web/src/test/core/mcp/remote/remoteClientTransport.test.ts index dcc86a37..34a08faa 100644 --- a/clients/web/src/test/core/mcp/remote/remoteClientTransport.test.ts +++ b/clients/web/src/test/core/mcp/remote/remoteClientTransport.test.ts @@ -342,6 +342,7 @@ describe("RemoteClientTransport", () => { connectionTimeout: 0, requestTimeout: 0, taskTtl: 0, + roots: [], }; const transport = new RemoteClientTransport( { diff --git a/clients/web/src/test/core/mcp/serverList.test.ts b/clients/web/src/test/core/mcp/serverList.test.ts index bf6de2c5..d029220c 100644 --- a/clients/web/src/test/core/mcp/serverList.test.ts +++ b/clients/web/src/test/core/mcp/serverList.test.ts @@ -1,5 +1,6 @@ import { describe, it, expect } from "vitest"; import { + cleanRoots, DEFAULT_SEED_CONFIG, expectedSecretFields, extractSecretsFromStored, @@ -74,6 +75,31 @@ describe("normalizeServerType", () => { }); }); +describe("cleanRoots", () => { + it("drops blank-uri rows and trims/drops blank names", () => { + expect( + cleanRoots([ + { uri: "file:///a", name: " Alpha " }, + { uri: "file:///b", name: " " }, + { uri: " " }, + { uri: "" }, + ]), + ).toEqual([{ uri: "file:///a", name: "Alpha" }, { uri: "file:///b" }]); + }); + + it("preserves non-form fields (e.g. _meta) on surviving rows", () => { + expect( + cleanRoots([ + { uri: "file:///a", name: "Alpha", _meta: { k: 1 } }, + { uri: "file:///b", _meta: { k: 2 } }, + ]), + ).toEqual([ + { uri: "file:///a", name: "Alpha", _meta: { k: 1 } }, + { uri: "file:///b", _meta: { k: 2 } }, + ]); + }); +}); + describe("mcpConfigToServerEntries", () => { it("uses the map key as both id and name", () => { const cfg: MCPConfig = { @@ -203,6 +229,8 @@ describe("serverEntriesToMcpConfig", () => { requestTimeout: 0, // Absent taskTtl on disk → product default in memory (for the form) taskTtl: 60000, + // Absent roots on disk → empty list in memory (for the form) + roots: [], // Nested oauth on disk → flat oauthClientId in memory oauthClientId: "the-client", }); @@ -275,6 +303,7 @@ describe("serverEntriesToMcpConfig", () => { connectionTimeout: 0, requestTimeout: 0, taskTtl: 0, + roots: [], }, connection: { status: "disconnected" }, }, @@ -299,6 +328,7 @@ describe("serverEntriesToMcpConfig", () => { connectionTimeout: 0, requestTimeout: 0, taskTtl: 0, + roots: [], }, connection: { status: "disconnected" }, }, @@ -310,6 +340,90 @@ describe("serverEntriesToMcpConfig", () => { expect(stored).not.toHaveProperty("oauth"); expect(stored).not.toHaveProperty("headers"); expect(stored).not.toHaveProperty("metadata"); + expect(stored).not.toHaveProperty("roots"); + }); + + it("round-trips roots (uri + optional name) onto the top-level disk field", () => { + const entries: ServerEntry[] = [ + { + id: "alpha", + name: "alpha", + config: { type: "stdio", command: "node" }, + settings: { + headers: [], + metadata: [], + connectionTimeout: 0, + requestTimeout: 0, + taskTtl: 0, + roots: [ + { uri: "file:///project", name: "Project" }, + { uri: "file:///tmp" }, + ], + }, + connection: { status: "disconnected" }, + }, + ]; + const stored = serverEntriesToMcpConfig(entries).mcpServers.alpha; + expect(stored?.roots).toEqual([ + { uri: "file:///project", name: "Project" }, + { uri: "file:///tmp" }, + ]); + // Disk → memory lifts the same roots back onto settings. + const [entry] = mcpConfigToServerEntries({ + mcpServers: { alpha: stored! }, + }); + expect(entry?.settings?.roots).toEqual([ + { uri: "file:///project", name: "Project" }, + { uri: "file:///tmp" }, + ]); + }); + + it("drops empty-uri root rows and blank names on serialize", () => { + // The form leaves a blank row when the user clicks 'Add Root' but hasn't + // typed a URI yet; a cleared optional name should not write `name: ""`. + const entries: ServerEntry[] = [ + { + id: "alpha", + name: "alpha", + config: { type: "stdio", command: "node" }, + settings: { + headers: [], + metadata: [], + connectionTimeout: 0, + requestTimeout: 0, + taskTtl: 0, + roots: [ + { uri: "file:///keep", name: " " }, + { uri: " " }, + { uri: "" }, + ], + }, + connection: { status: "disconnected" }, + }, + ]; + const stored = serverEntriesToMcpConfig(entries).mcpServers.alpha; + expect(stored?.roots).toEqual([{ uri: "file:///keep" }]); + }); + + it("omits the roots field entirely when no rows survive filtering", () => { + const entries: ServerEntry[] = [ + { + id: "alpha", + name: "alpha", + config: { type: "stdio", command: "node" }, + settings: { + headers: [], + metadata: [], + connectionTimeout: 0, + requestTimeout: 0, + taskTtl: 0, + roots: [{ uri: "" }], + }, + connection: { status: "disconnected" }, + }, + ]; + const stored = serverEntriesToMcpConfig(entries).mcpServers.alpha; + expect(stored).not.toHaveProperty("roots"); }); it("preserves insertion order on serialize", () => { diff --git a/clients/web/src/test/core/react/useServers.test.tsx b/clients/web/src/test/core/react/useServers.test.tsx index 3661a11f..9869da47 100644 --- a/clients/web/src/test/core/react/useServers.test.tsx +++ b/clients/web/src/test/core/react/useServers.test.tsx @@ -392,6 +392,7 @@ describe("useServers", () => { connectionTimeout: 5000, requestTimeout: 30000, taskTtl: 30000, + roots: [], }); }); @@ -402,6 +403,7 @@ describe("useServers", () => { connectionTimeout: 5000, requestTimeout: 30000, taskTtl: 30000, + roots: [], }); }); const stored = readConfig(h.configPath).mcpServers @@ -433,6 +435,7 @@ describe("useServers", () => { connectionTimeout: 0, requestTimeout: 0, taskTtl: 0, + roots: [], }); }), ).rejects.toThrow(/not found/); @@ -481,6 +484,8 @@ describe("useServers", () => { requestTimeout: 0, // Absent taskTtl on disk reads back as the product default for the form. taskTtl: 60000, + // Absent roots on disk reads back as an empty list for the form. + roots: [], }); }); diff --git a/clients/web/src/test/integration/mcp/inspectorClient.test.ts b/clients/web/src/test/integration/mcp/inspectorClient.test.ts index 61c29693..5b5af9d5 100644 --- a/clients/web/src/test/integration/mcp/inspectorClient.test.ts +++ b/clients/web/src/test/integration/mcp/inspectorClient.test.ts @@ -320,6 +320,7 @@ describe("InspectorClient", () => { connectionTimeout: 50, requestTimeout: 0, taskTtl: 0, + roots: [], }, }, ); diff --git a/clients/web/src/test/integration/mcp/node/transport.test.ts b/clients/web/src/test/integration/mcp/node/transport.test.ts index 56abb830..060a93f7 100644 --- a/clients/web/src/test/integration/mcp/node/transport.test.ts +++ b/clients/web/src/test/integration/mcp/node/transport.test.ts @@ -217,6 +217,7 @@ describe("Transport", () => { connectionTimeout: 0, requestTimeout: 0, taskTtl: 0, + roots: [], }; const fetchRequests: FetchRequestEntryBase[] = []; @@ -269,6 +270,7 @@ describe("Transport", () => { connectionTimeout: 0, requestTimeout: 0, taskTtl: 0, + roots: [], }; const fetchRequests: FetchRequestEntryBase[] = []; @@ -311,6 +313,7 @@ describe("Transport", () => { connectionTimeout: 0, requestTimeout: 0, taskTtl: 0, + roots: [], }; const result = createTransportNode(config, { settings }); // Just exercise the empty-headers path — no transport construction diff --git a/clients/web/src/test/integration/mcp/remote/servers-route.test.ts b/clients/web/src/test/integration/mcp/remote/servers-route.test.ts index 734b2e79..ee52c4a6 100644 --- a/clients/web/src/test/integration/mcp/remote/servers-route.test.ts +++ b/clients/web/src/test/integration/mcp/remote/servers-route.test.ts @@ -829,6 +829,8 @@ describe("/api/servers routes", () => { requestTimeout: 60000, // Bad: array instead of object → drop oauth: ["not", "an", "object"], + // Bad: entry missing a string `uri` → drop the whole roots field + roots: [{ name: "no-uri" }], }, }, }), @@ -847,6 +849,114 @@ describe("/api/servers routes", () => { expect(fetched).not.toHaveProperty("metadata"); expect(fetched).not.toHaveProperty("connectionTimeout"); expect(fetched).not.toHaveProperty("oauth"); + expect(fetched).not.toHaveProperty("roots"); + }); + + it("round-trips roots (uri + optional name) on PUT and drops empty-uri rows", async () => { + writeFileSync( + h.configPath, + JSON.stringify({ + mcpServers: { + "roots-srv": { type: "stdio", command: "node" }, + }, + }), + ); + const res = await fetch(`${h.baseUrl}/api/servers/roots-srv`, { + method: "PUT", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ + config: { type: "stdio", command: "node" }, + settings: { + headers: [], + metadata: [], + connectionTimeout: 0, + requestTimeout: 0, + roots: [ + { uri: "file:///project", name: "Project" }, + { uri: "file:///tmp" }, + // Empty-uri row left mid-edit by the form → dropped on write. + { uri: "" }, + ], + }, + }), + }); + expect(res.status).toBe(200); + const stored = readConfig(h.configPath).mcpServers[ + "roots-srv" + ] as unknown as Record; + expect(stored.roots).toEqual([ + { uri: "file:///project", name: "Project" }, + { uri: "file:///tmp" }, + ]); + + // The GET round-trip surfaces the same roots back to the form. + const getRes = await fetch(`${h.baseUrl}/api/servers`); + const getBody = (await getRes.json()) as { + mcpServers: Record>; + }; + expect(getBody.mcpServers["roots-srv"]!.roots).toEqual([ + { uri: "file:///project", name: "Project" }, + { uri: "file:///tmp" }, + ]); + }); + + it("omits the roots field on disk when all rows are empty", async () => { + writeFileSync( + h.configPath, + JSON.stringify({ + mcpServers: { + "roots-empty": { type: "stdio", command: "node" }, + }, + }), + ); + const res = await fetch(`${h.baseUrl}/api/servers/roots-empty`, { + method: "PUT", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ + config: { type: "stdio", command: "node" }, + settings: { + headers: [], + metadata: [], + connectionTimeout: 0, + requestTimeout: 0, + roots: [{ uri: " " }], + }, + }), + }); + expect(res.status).toBe(200); + const stored = readConfig(h.configPath).mcpServers[ + "roots-empty" + ] as unknown as Record; + expect(stored).not.toHaveProperty("roots"); + }); + + it("rejects a malformed roots shape with 400", async () => { + writeFileSync( + h.configPath, + JSON.stringify({ + mcpServers: { + "roots-bad": { type: "stdio", command: "node" }, + }, + }), + ); + const res = await fetch(`${h.baseUrl}/api/servers/roots-bad`, { + method: "PUT", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ + config: { type: "stdio", command: "node" }, + settings: { + headers: [], + metadata: [], + connectionTimeout: 0, + requestTimeout: 0, + // uri must be a string. + roots: [{ uri: 42 }], + }, + }), + }); + expect(res.status).toBe(400); + const body = (await res.json()) as { error?: string }; + expect(body.error).toMatch(/settings\.roots/); }); it("loads a hand-edited file with top-level Claude Code-style `headers` (interop with `.mcp.json`)", async () => { diff --git a/clients/web/src/test/integration/mcp/remote/transport.test.ts b/clients/web/src/test/integration/mcp/remote/transport.test.ts index 61dfff7f..e3082a43 100644 --- a/clients/web/src/test/integration/mcp/remote/transport.test.ts +++ b/clients/web/src/test/integration/mcp/remote/transport.test.ts @@ -411,6 +411,7 @@ describe("Remote transport e2e", () => { connectionTimeout: 0, requestTimeout: 0, taskTtl: 0, + roots: [], }; const createTransport = createRemoteTransport({ baseUrl, authToken }); diff --git a/core/mcp/remote/node/server.ts b/core/mcp/remote/node/server.ts index abb6c730..55c7659c 100644 --- a/core/mcp/remote/node/server.ts +++ b/core/mcp/remote/node/server.ts @@ -870,6 +870,21 @@ export function createRemoteApp( } return true; }; + // A roots array: each entry must have a string `uri` and, when present, a + // string `name` (SDK `Root`). Other keys (e.g. `_meta`) pass through — we + // only gate the two fields the Inspector reads/writes. + const isRootArray = ( + v: unknown, + ): v is { uri: string; name?: string }[] => { + if (!Array.isArray(v)) return false; + return v.every((e) => { + if (e === null || typeof e !== "object") return false; + const o = e as Record; + if (typeof o.uri !== "string") return false; + if (o.name !== undefined && typeof o.name !== "string") return false; + return true; + }); + }; // `Number.isFinite` rejects `Infinity` and `NaN` as well as non-numbers, // matching the write-side semantics in `validateSettings`. A hand-edited // file with `connectionTimeout: Infinity` would otherwise pass the guard @@ -954,6 +969,13 @@ export function createRemoteApp( ); delete valObj.oauth; } + if ("roots" in valObj && !isRootArray(valObj.roots)) { + logWarn( + { route: "/api/servers", id, droppedKey: "roots" }, + "Dropping malformed `roots` field — expected `Array<{ uri: string, name?: string }>`.", + ); + delete valObj.roots; + } out[id] = normalizeServerType( valObj as Record & { type?: string }, @@ -1096,6 +1118,16 @@ export function createRemoteApp( return { ok: false, error: `settings.${optional} must be a string` }; } } + // roots is optional on the wire (older clients won't send it); when + // present it must be an array of `{ uri, name? }`. Reuses the same + // module-scope `isRootArray` guard the read path applies in + // `normalizeMcpServers`, so the two paths can't drift. + if (obj.roots !== undefined && !isRootArray(obj.roots)) { + return { + ok: false, + error: "settings.roots must be an array of { uri, name? }", + }; + } // Build the validated value from explicitly named fields rather than // casting the raw object through. Unknown keys silently drop so a // misconfigured client can't smuggle stowaways onto disk, and consumers @@ -1114,6 +1146,11 @@ export function createRemoteApp( // to disk for a client that didn't send one. taskTtl: typeof obj.taskTtl === "number" ? obj.taskTtl : DEFAULT_TASK_TTL_MS, + // Absent → empty list, matching the read side + // (storedFieldsToInspectorSettings). Empty rows are dropped on the way + // to disk by inspectorSettingsToStoredFields, so an empty array here + // writes no spurious `roots` field. + roots: isRootArray(obj.roots) ? obj.roots : [], }; if (typeof obj.oauthClientId === "string" && obj.oauthClientId !== "") { value.oauthClientId = obj.oauthClientId; diff --git a/core/mcp/serverList.ts b/core/mcp/serverList.ts index ce8de3be..43ac296d 100644 --- a/core/mcp/serverList.ts +++ b/core/mcp/serverList.ts @@ -6,6 +6,7 @@ */ import { DEFAULT_TASK_TTL_MS } from "./types.js"; +import type { Root } from "@modelcontextprotocol/sdk/types.js"; import type { InspectorServerSettings, MCPConfig, @@ -57,6 +58,27 @@ export function normalizeServerType( return { ...config, type: normalizedType } as MCPServerConfig; } +/** + * Normalize the form's controlled root rows into the shape the Inspector + * advertises and persists: drop rows whose `uri` is blank (the form leaves a + * new row empty mid-edit) and drop a blank/whitespace `name`. Any other fields + * a root carries (e.g. `_meta` from a hand-edited `mcp.json`) are preserved — + * only `uri`/`name` are normalized. Shared by the settings → disk converter + * (`inspectorSettingsToStoredFields`) and the web client's connect-time + + * `setRoots` wiring so the roots told to the server match what hits disk. + */ +export function cleanRoots(roots: Root[]): Root[] { + return roots + .filter((r) => r.uri.trim() !== "") + .map((r) => { + const trimmedName = r.name?.trim(); + // Strip `name` off the carried-through rest so a cleared optional name + // doesn't persist as `name: ""`; re-add it only when non-empty. + const { name: _name, ...rest } = r; + return trimmedName ? { ...rest, name: trimmedName } : rest; + }); +} + /** * The Inspector-extension fields that live as direct keys on a `StoredMCPServer` * (post-#1358) — split out from `MCPServerConfig` so both directions of the @@ -71,6 +93,7 @@ type StoredInspectorFields = Pick< | "requestTimeout" | "taskTtl" | "oauth" + | "roots" >; /** @@ -94,7 +117,8 @@ export function storedFieldsToInspectorSettings( stored.connectionTimeout !== undefined || stored.requestTimeout !== undefined || stored.taskTtl !== undefined || - stored.oauth !== undefined; + stored.oauth !== undefined || + stored.roots !== undefined; if (!hasAny) return undefined; const headersPairs: { key: string; value: string }[] = stored.headers @@ -109,6 +133,10 @@ export function storedFieldsToInspectorSettings( // Unlike the timeouts (0 = "SDK default"), task TTL has a concrete product // default so the form shows it and "Run as task" has a value to send. taskTtl: stored.taskTtl ?? DEFAULT_TASK_TTL_MS, + // Defaults to an empty list so the form always has a concrete array to + // render controlled rows from. An absent on-disk `roots` reads back as + // `[]`, which `inspectorSettingsToStoredFields` then omits on write. + roots: stored.roots ?? [], }; // Truthiness drops empty-string OAuth fields — mirrors the write-side // coercion in `validateSettings` (server.ts) so a round-trip can't @@ -179,6 +207,14 @@ export function inspectorSettingsToStoredFields( out.oauth = oauthFields; } + // Drop empty-uri rows / blank names via the shared normalizer; omit the + // field entirely when nothing survives, keeping the diff minimal for entries + // that never configured roots. + const rootsFiltered = cleanRoots(settings.roots); + if (rootsFiltered.length > 0) { + out.roots = rootsFiltered; + } + return out; } @@ -203,6 +239,7 @@ const INSPECTOR_FIELD_KEY_MAP = { requestTimeout: true, taskTtl: true, oauth: true, + roots: true, } as const satisfies Record; export const INSPECTOR_FIELD_KEYS = new Set( diff --git a/core/mcp/types.ts b/core/mcp/types.ts index b92c1bc4..29bf50c0 100644 --- a/core/mcp/types.ts +++ b/core/mcp/types.ts @@ -115,6 +115,14 @@ export type StoredMCPServer = MCPServerConfig & { clientSecret?: string; scopes?: string; }; + /** + * Filesystem/URI roots advertised to the server via the `roots` client + * capability. Inspector-specific (no analog in the broader mcp.json + * ecosystem). Each root is the SDK `Root` shape `{ uri, name? }`; unlike v1 + * (URI-only), the optional `name` round-trips here. Persisted as-is on disk + * and lifted onto `InspectorServerSettings.roots` when read into memory. + */ + roots?: Root[]; }; export interface MCPConfig { @@ -364,6 +372,13 @@ export interface InspectorServerSettings { oauthClientId?: string; oauthClientSecret?: string; oauthScopes?: string; + /** + * Roots advertised to the server via the `roots` client capability. Each + * root carries a required `uri` and an optional `name` (SDK `Root`). The + * form edits these as controlled rows; empty-uri rows are dropped on + * persist (see `inspectorSettingsToStoredFields`). + */ + roots: Root[]; } /**