From 6cf48a00fe9301ceecb0f2a42dccc2f4694c5e92 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Sat, 23 May 2026 16:23:04 -0400 Subject: [PATCH 01/18] feat(servers): persist per-server settings to mcp.json (#1352) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add optional `settings` node alongside the transport config in each `~/.mcp-inspector/mcp.json` entry. Round-trips through `mcpConfigToServerEntries` / `serverEntriesToMcpConfig` and the `/api/servers` POST/PUT routes. - Drop `MCPServerConfig.headers` from `SseServerConfig` / `StreamableHttpServerConfig` and remove the headers textarea from `ServerConfigModal`. Custom headers are now entered only in `ServerSettingsForm` and reach the wire via `settings.headers`. - Wire `ServerSettingsModal` into `App.tsx`; saves dispatch through a new `useServers.updateServerSettings` mutator. `updateServer` carries the existing settings across config-only saves so they aren't dropped. - Apply settings on the *first* outbound request after a server loads — no need to open the settings form first: - `settings.headers` → SSE / streamable-http wire headers. - `settings.metadata` → `InspectorClientOptions.defaultMetadata`, merged into every outgoing `_meta` (call-time keys win). - `settings.requestTimeout` → `InspectorClientOptions.timeout`. - `settings.connectionTimeout` → Promise.race wrapper around connect(). - `settings.oauth*` → pre-seeded OAuth client credentials. - `RemoteConnectRequest` carries `settings` so the remote backend can source transport headers from the same source as the local path. - `oauthClientSecret` is persisted in `mcp.json` alongside stdio `env` values, both protected by the file's `0o600` permission. - Update `specification/v2_servers_file.md` with the new shape and the removal of the legacy headers field. Tests: round-trip in `serverList.test.ts`; persistence on POST/PUT in `servers-route.test.ts`; first-connect header injection in `transport.test.ts`; `defaultMetadata` merging in `inspectorClient.test.ts`; `useServers.updateServerSettings` + `updateServer` settings-preservation in `useServers.test.tsx`. Co-Authored-By: Claude Opus 4.7 (1M context) --- clients/web/server/web-server-config.ts | 12 +- clients/web/src/App.tsx | 108 +++++++++++++- .../ServerConfigModal.stories.tsx | 4 +- .../ServerConfigModal.test.tsx | 34 +---- .../ServerConfigModal/ServerConfigModal.tsx | 97 +++--------- .../web/src/test/core/mcp/node/config.test.ts | 53 +------ .../web/src/test/core/mcp/serverList.test.ts | 65 +++++++- .../src/test/core/react/useServers.test.tsx | 107 +++++++++++++ .../integration/mcp/inspectorClient.test.ts | 109 ++++++++++++++ .../integration/mcp/node/transport.test.ts | 123 +++++++++++++++ .../mcp/remote/servers-route.test.ts | 115 ++++++++++++++ .../server/web-server-config.test.ts | 5 - core/mcp/inspectorClient.ts | 140 +++++++++--------- core/mcp/node/config.ts | 28 +--- core/mcp/node/transport.ts | 29 +++- core/mcp/remote/createRemoteTransport.ts | 1 + core/mcp/remote/node/server.ts | 71 +++++++-- core/mcp/remote/remoteClientTransport.ts | 10 +- core/mcp/remote/types.ts | 12 +- core/mcp/serverList.ts | 38 +++-- core/mcp/types.ts | 44 +++++- core/react/useServers.ts | 50 ++++++- specification/v2_servers_file.md | 38 ++++- 23 files changed, 991 insertions(+), 302 deletions(-) diff --git a/clients/web/server/web-server-config.ts b/clients/web/server/web-server-config.ts index f219554f4..f3e834fdb 100644 --- a/clients/web/server/web-server-config.ts +++ b/clients/web/server/web-server-config.ts @@ -97,7 +97,6 @@ export function webServerConfigToInitialPayload( return { defaultTransport: "sse", defaultServerUrl: mc.url, - defaultHeaders: mc.headers ?? undefined, defaultEnvironment, }; } @@ -105,21 +104,18 @@ export function webServerConfigToInitialPayload( return { defaultTransport: "streamable-http", defaultServerUrl: mc.url, - defaultHeaders: mc.headers ?? undefined, defaultEnvironment, }; } // Forward-compat fallback: if a future SDK ships a new transport type the // launcher hasn't taught us about yet, prefer streamable-http (the most - // permissive shape with `url` + optional `headers`) over throwing. Worst - // case the UI shows the URL the user supplied; throwing here would deny - // the user a hint at startup. Add a real branch above once the type is - // known. - const c = mc as unknown as { url: string; headers?: Record }; + // permissive shape with `url`) over throwing. Worst case the UI shows the + // URL the user supplied; throwing here would deny the user a hint at + // startup. Add a real branch above once the type is known. + const c = mc as unknown as { url: string }; return { defaultTransport: "streamable-http", defaultServerUrl: c.url, - defaultHeaders: c.headers, defaultEnvironment, }; } diff --git a/clients/web/src/App.tsx b/clients/web/src/App.tsx index 4df0a19b3..5acc9fa6d 100644 --- a/clients/web/src/App.tsx +++ b/clients/web/src/App.tsx @@ -10,6 +10,7 @@ import type { AppBridge } from "@modelcontextprotocol/ext-apps/app-bridge"; import { InspectorClient } from "@inspector/core/mcp/index.js"; import type { JsonValue } from "@inspector/core/mcp/index.js"; import type { + InspectorServerSettings, MCPServerConfig, MessageEntry, ServerEntry, @@ -45,6 +46,7 @@ import { ServerConfigModal, type ServerConfigModalMode, } from "./components/groups/ServerConfigModal/ServerConfigModal"; +import { ServerSettingsModal } from "./components/groups/ServerSettingsModal/ServerSettingsModal"; import { ServerRemoveConfirmModal } from "./components/groups/ServerRemoveConfirmModal/ServerRemoveConfirmModal"; import { downloadJsonFile } from "./lib/downloadFile"; import { createWebEnvironment } from "./lib/environmentFactory"; @@ -135,7 +137,13 @@ function App() { // Server list — sourced from ~/.mcp-inspector/mcp.json via the backend's // `/api/servers` routes. First-launch seeds are written by the backend when // the file is absent, so this hook returns a non-empty list on first load. - const { servers, addServer, updateServer, removeServer } = useServers({ + const { + servers, + addServer, + updateServer, + updateServerSettings, + removeServer, + } = useServers({ baseUrl: typeof window !== "undefined" ? window.location.origin @@ -149,6 +157,9 @@ function App() { mode: ServerConfigModalMode; targetId?: string; } | null>(null); + const [settingsModalTargetId, setSettingsModalTargetId] = useState< + string | undefined + >(undefined); const [removeTarget, setRemoveTarget] = useState(null); // The active connection target. `null` between sessions; set as soon as @@ -336,6 +347,31 @@ function App() { getAuthToken(), redirectUrlProvider, ); + const settings = server.settings; + // Flatten the persisted settings into the InspectorClient options shape. + // Empty / zero values stay unset so the SDK defaults apply. + const defaultMetadata = settings?.metadata + ? Object.fromEntries( + settings.metadata + .filter((m) => m.key.trim() !== "") + .map((m) => [m.key, m.value]), + ) + : undefined; + const oauth = + settings && + (settings.oauthClientId || + settings.oauthClientSecret || + settings.oauthScopes) + ? { + ...(settings.oauthClientId && { + clientId: settings.oauthClientId, + }), + ...(settings.oauthClientSecret && { + clientSecret: settings.oauthClientSecret, + }), + ...(settings.oauthScopes && { scope: settings.oauthScopes }), + } + : undefined; const client = new InspectorClient(server.config, { environment, // The Tasks tab needs the receiver-task pipeline; the @@ -344,6 +380,16 @@ 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 }, + ...(settings && + settings.requestTimeout > 0 && { + timeout: settings.requestTimeout, + }), + ...(defaultMetadata && + Object.keys(defaultMetadata).length > 0 && { + defaultMetadata, + }), + ...(oauth && { oauth }), + ...(settings && { serverSettings: settings }), }); setInspectorClient(client); @@ -405,8 +451,29 @@ function App() { setErrorMessage(undefined); connectStartRef.current = Date.now(); + const connectionTimeout = target.settings?.connectionTimeout ?? 0; try { - await client.connect(); + if (connectionTimeout > 0) { + // Race the handshake against a hard timeout. The MCP SDK has no + // connect-time timeout option, so we wrap here and tear the + // transport down on timeout so it doesn't leak. + const connectClient = client; + let timer: ReturnType | undefined; + const timeout = new Promise((_, reject) => { + timer = setTimeout(() => { + reject( + new Error(`Connection timed out after ${connectionTimeout} ms`), + ); + }, connectionTimeout); + }); + try { + await Promise.race([connectClient.connect(), timeout]); + } finally { + if (timer) clearTimeout(timer); + } + } else { + await client.connect(); + } } catch (err) { // Handshake-only. A mid-session transport failure transitions the // client status to "error" without rejecting any pending promise, @@ -415,6 +482,9 @@ function App() { connectStartRef.current = undefined; const message = err instanceof Error ? err.message : String(err); setErrorMessage(message); + // On timeout (or any handshake failure) drop the transport so the + // next toggle starts clean. + await client.disconnect().catch(() => {}); } }, [ @@ -705,6 +775,32 @@ function App() { return servers.find((s) => s.id === configModal.targetId); }, [configModal, servers]); + const settingsModalTarget = useMemo(() => { + if (!settingsModalTargetId) return undefined; + return servers.find((s) => s.id === settingsModalTargetId); + }, [settingsModalTargetId, servers]); + + // Stable starting shape for entries that have no `settings` node yet — the + // form needs concrete arrays / numbers to render. + const settingsModalValue = useMemo(() => { + return ( + settingsModalTarget?.settings ?? { + headers: [], + metadata: [], + connectionTimeout: 0, + requestTimeout: 0, + } + ); + }, [settingsModalTarget]); + + const onSettingsChange = useCallback( + (next: InspectorServerSettings) => { + if (!settingsModalTargetId) return; + void updateServerSettings(settingsModalTargetId, next); + }, + [settingsModalTargetId, updateServerSettings], + ); + // The Resources screen needs `isSubscribed` to flip the Subscribe button // label to "Unsubscribe". Derive it from the live subscriptions list rather // than threading it through every setReadResourceState site — that way the @@ -756,7 +852,7 @@ function App() { onServerImportJson={todoNoop} onServerExport={onServerExport} onServerInfo={todoNoop} - onServerSettings={todoNoop} + onServerSettings={(id) => setSettingsModalTargetId(id)} onServerEdit={(id) => setConfigModal({ mode: "edit", targetId: id })} onServerClone={(id) => setConfigModal({ mode: "clone", targetId: id })} onServerRemove={(id) => { @@ -805,6 +901,12 @@ function App() { onClose={() => setConfigModal(null)} onSubmit={onConfigSubmit} /> + setSettingsModalTargetId(undefined)} + onSettingsChange={onSettingsChange} + /> = { @@ -120,6 +119,7 @@ export const EditSse: Story = { play: async ({ canvasElement }) => { const body = within(canvasElement.ownerDocument.body); await expect(await body.findByLabelText(/^URL/)).toBeInTheDocument(); - await expect(body.getByLabelText(/Headers/i)).toBeInTheDocument(); + // Headers are no longer entered here — they live in ServerSettingsForm. + await expect(body.queryByLabelText(/Headers/i)).toBeNull(); }, }; diff --git a/clients/web/src/components/groups/ServerConfigModal/ServerConfigModal.test.tsx b/clients/web/src/components/groups/ServerConfigModal/ServerConfigModal.test.tsx index d07a3dacc..6b75611eb 100644 --- a/clients/web/src/components/groups/ServerConfigModal/ServerConfigModal.test.tsx +++ b/clients/web/src/components/groups/ServerConfigModal/ServerConfigModal.test.tsx @@ -95,7 +95,7 @@ describe("ServerConfigModal", () => { // open reliably; we exercise the sse rendering branch by seeding the form // with an sse initialConfig (the same code path the Select onChange takes). - it("renders url + headers (and hides stdio fields) when transport is sse", () => { + it("renders url (and hides stdio fields) when transport is sse", () => { renderWithMantine( { />, ); expect(screen.getByLabelText(/^URL/)).toBeInTheDocument(); - expect(screen.getByLabelText(/Headers/i)).toBeInTheDocument(); + expect(screen.queryByLabelText(/Headers/i)).not.toBeInTheDocument(); expect(screen.queryByLabelText(/^Command/)).not.toBeInTheDocument(); }); - it("submits an sse config with parsed headers", async () => { + it("submits an sse config with just the url (headers move to the settings form)", async () => { const user = userEvent.setup(); const props = base(); renderWithMantine( @@ -122,38 +122,16 @@ describe("ServerConfigModal", () => { ); await user.type(screen.getByLabelText(/^URL/), "https://x.test/sse"); - await user.type( - screen.getByLabelText(/Headers/i), - "Authorization: Bearer xxx", - ); await user.click(screen.getByRole("button", { name: /^Save$/ })); await waitFor(() => expect(props.onSubmit).toHaveBeenCalledOnce()); expect(props.onSubmit).toHaveBeenCalledWith("remote", { type: "sse", url: "https://x.test/sse", - headers: { Authorization: "Bearer xxx" }, }); }); - it("rejects malformed header lines on sse submission", async () => { - const user = userEvent.setup(); - const props = base(); - renderWithMantine( - , - ); - await user.type(screen.getByLabelText(/Headers/i), "BAD_HEADER_LINE"); - await user.click(screen.getByRole("button", { name: /^Save$/ })); - expect(screen.getByRole("alert")).toHaveTextContent(/Invalid header/i); - expect(props.onSubmit).not.toHaveBeenCalled(); - }); - - it("submits a streamable-http config (with no headers)", async () => { + it("submits a streamable-http config", async () => { const user = userEvent.setup(); const props = base(); renderWithMantine( @@ -172,7 +150,7 @@ describe("ServerConfigModal", () => { }); }); - it("loads initial headers + url from a streamable-http config", () => { + it("loads the url from a streamable-http config", () => { renderWithMantine( { initialConfig={{ type: "streamable-http", url: "https://x.test/mcp", - headers: { "X-Trace": "abc" }, }} />, ); expect(screen.getByLabelText(/^URL/)).toHaveValue("https://x.test/mcp"); - expect(screen.getByLabelText(/Headers/i)).toHaveValue("X-Trace: abc"); }); it("rejects an empty URL on sse submission", async () => { diff --git a/clients/web/src/components/groups/ServerConfigModal/ServerConfigModal.tsx b/clients/web/src/components/groups/ServerConfigModal/ServerConfigModal.tsx index 32be68268..ca7970433 100644 --- a/clients/web/src/components/groups/ServerConfigModal/ServerConfigModal.tsx +++ b/clients/web/src/components/groups/ServerConfigModal/ServerConfigModal.tsx @@ -11,9 +11,7 @@ import { } from "@mantine/core"; import type { MCPServerConfig, - SseServerConfig, StdioServerConfig, - StreamableHttpServerConfig, } from "@inspector/core/mcp/types.js"; /** Allowed id pattern — mirrors validateStoreId in core/storage/store-io.ts */ @@ -47,7 +45,6 @@ interface FormState { cwd: string; // sse / streamable-http url: string; - headersText: string; } const SectionStack = Stack.withProps({ gap: "md" }); @@ -77,7 +74,6 @@ function configToFormState( envText: "", cwd: "", url: "", - headersText: "", }; } if (transport === "stdio") { @@ -92,11 +88,13 @@ function configToFormState( .join("\n"), cwd: c.cwd ?? "", url: "", - headersText: "", }; } - // sse / streamable-http - const c = initialConfig as SseServerConfig | StreamableHttpServerConfig; + // sse / streamable-http — custom headers live in ServerSettingsForm now. + const url = + initialConfig.type === "sse" || initialConfig.type === "streamable-http" + ? initialConfig.url + : ""; return { id, transport, @@ -104,10 +102,7 @@ function configToFormState( argsText: "", envText: "", cwd: "", - url: c.url ?? "", - headersText: Object.entries(c.headers ?? {}) - .map(([k, v]) => `${k}: ${v}`) - .join("\n"), + url: url ?? "", }; } @@ -139,8 +134,7 @@ function parseEnv(raw: string): } const key = line.slice(0, eq).trim(); // env values preserve trailing whitespace — they're shell-style strings - // where spaces / tabs can be load-bearing; header values are HTTP-style - // and parseHeaders below trims them per RFC 7230 §3.2.4. + // where spaces / tabs can be load-bearing. const value = line.slice(eq + 1); if (!key) return { ok: false, error: `Invalid env line "${line}". Empty key.` }; @@ -149,38 +143,6 @@ function parseEnv(raw: string): return { ok: true, value: out }; } -function parseHeaders(raw: string): - | { - ok: true; - value: Record; - } - | { - ok: false; - error: string; - } { - const out: Record = {}; - const lines = raw - .split("\n") - .map((s) => s.trim()) - .filter((s) => s.length > 0); - for (const line of lines) { - const colon = line.indexOf(":"); - if (colon <= 0) { - return { - ok: false, - error: `Invalid header line "${line}". Use "Header-Name: value".`, - }; - } - const key = line.slice(0, colon).trim(); - const value = line.slice(colon + 1).trim(); - if (!key) { - return { ok: false, error: `Invalid header line "${line}". Empty name.` }; - } - out[key] = value; - } - return { ok: true, value: out }; -} - export function ServerConfigModal({ opened, mode, @@ -247,14 +209,14 @@ export function ServerConfigModal({ if (!form.url.trim()) { return { ok: false, error: "URL is required for sse / streamable-http." }; } - const headers = parseHeaders(form.headersText); - if (!headers.ok) return headers; const base = { url: form.url.trim() }; - const config: SseServerConfig | StreamableHttpServerConfig = + // Custom headers live in ServerSettingsForm now (persisted under + // settings.headers on the entry); the SSE / streamable-http config here + // only carries the canonical transport fields. + const config: MCPServerConfig = form.transport === "sse" ? { type: "sse", ...base } : { type: "streamable-http", ...base }; - if (Object.keys(headers.value).length > 0) config.headers = headers.value; return { ok: true, config }; } @@ -384,32 +346,17 @@ export function ServerConfigModal({ /> ) : ( - <> - { - const next = e.currentTarget.value; - setForm((f) => ({ ...f, url: next })); - }} - required - disabled={submitting} - /> -