From b3aedf62cdd92fb6e48140612c97ff6a0a3e57fb Mon Sep 17 00:00:00 2001 From: cliffhall Date: Sun, 16 Aug 2026 18:31:01 -0400 Subject: [PATCH 01/13] fix(web): name an empty tool result instead of "No results yet" (#1860) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Tools screen mounts the Results panel only once a call has produced a result, but the panel's zero-content branch rendered "No results yet" — the same wording it would use before anything had run. An empty `content` array with no `structuredContent` is a legal `CallToolResult`, so a call that had plainly completed reported itself as a call that never happened. That reads worst after a multi-round-trip (MRTR) sequence, which is how it was reported: the user answers one or more embedded elicitations, watches the Protocol tab group the exchange and mark it COMPLETE, and is then told there are no results yet. Nothing was lost — the result was stored and rendered; the panel simply had no wording for "completed, returned nothing". The panel now renders an "Empty result" alert saying the call completed and returned no content. This is the remaining half of the same gap #1908 closed for `structuredContent`-only results. test-servers: new `mrtr_empty` preset (`createMrtrEmptyResultTool`) — one elicitation round, then a complete result with `content: []` and no `structuredContent` — added to `mrtr-showcase-http.json` so the shape is reproducible by hand. README documents it. Signed-off-by: Cliff Hall Signed-off-by: cliffhall --- README.md | 15 ++++++- .../ToolResultPanel/ToolResultPanel.test.tsx | 15 +++++-- .../ToolResultPanel/ToolResultPanel.tsx | 15 ++++++- test-servers/configs/mrtr-showcase-http.json | 1 + test-servers/src/preset-registry.ts | 3 ++ test-servers/src/test-server-fixtures.ts | 44 +++++++++++++++++++ 6 files changed, 87 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index cb57dd35..d10c191f 100644 --- a/README.md +++ b/README.md @@ -140,7 +140,7 @@ Each config below is a ready-made server for exercising one feature by hand. Loa | ----------------------------------------- | -------------------------------------------------- | ---------------------------------------------------------------------- | | `mcp-app-http.json` **(legacy era)** | An MCP App (UI resource + app tool) in the Apps tab | [#1859](https://github.com/modelcontextprotocol/inspector/issues/1859) | | `modern-mrtr-http.json` | A single MRTR round-trip | — | -| `mrtr-showcase-http.json` | Every MRTR preset in one server | — | +| `mrtr-showcase-http.json` | Every MRTR preset in one server | [#1860](https://github.com/modelcontextprotocol/inspector/issues/1860) | | `modern-network-http.json` | Network tab: `Mcp-*` headers + error taxonomy | [#1628](https://github.com/modelcontextprotocol/inspector/issues/1628) | | `xmcpheader-modern-http.json` | Tools tab: `x-mcp-header` mirroring and exclusions | [#1632](https://github.com/modelcontextprotocol/inspector/issues/1632) | | `pagination-http.json` | Page-by-page list fetching | [#1721](https://github.com/modelcontextprotocol/inspector/issues/1721) | @@ -175,8 +175,21 @@ The Inspector drives MRTR manually (`inputRequired: { autoFulfill: false }`), so | `mrtr_sample` | Embedded sampling → the Sampling panel | | `mrtr_roots` | Embedded `roots/list`, auto-answered silently from configured roots (no modal) | | `mrtr_edge` | An `inputRequests`-only round, then a `requestState`-only round | +| `mrtr_empty` | Completes with an empty result — no `content`, no `structuredContent` | | `mrtr_loop` | Never completes → trips the `MRTR_MAX_ROUNDS` bound | +Run `mrtr_empty` and answer its single elicitation: the Protocol tab groups the +exchange as an MRTR conversation ending **COMPLETE**, and the Results panel says +**"Empty result — The tool call completed successfully and returned no content."** +On the broken build that same result rendered as **"No results yet"**, the panel's +pre-run placeholder ([#1860](https://github.com/modelcontextprotocol/inspector/issues/1860)) — +so a call the user had just watched succeed read as a call that never ran. An +empty `content` array with no `structuredContent` is a legal `CallToolResult`, +and the panel only ever mounts once a result exists, so the placeholder wording +could not be true there. (The neighbouring half of the same gap — a result whose +payload lives only in `structuredContent` — was closed by +[#1908](https://github.com/modelcontextprotocol/inspector/issues/1908).) + > The legacy `collect_elicitation` preset calls `server.elicitInput`, which errors on the 2026-07-28 leg — server→client requests aren't allowed there. MRTR is the modern replacement. #### Network tab — standardized headers and error taxonomy diff --git a/clients/web/src/components/groups/ToolResultPanel/ToolResultPanel.test.tsx b/clients/web/src/components/groups/ToolResultPanel/ToolResultPanel.test.tsx index 1be9f2c3..0820e01b 100644 --- a/clients/web/src/components/groups/ToolResultPanel/ToolResultPanel.test.tsx +++ b/clients/web/src/components/groups/ToolResultPanel/ToolResultPanel.test.tsx @@ -35,11 +35,20 @@ describe("ToolResultPanel", () => { expect(screen.getByText("boom")).toBeInTheDocument(); }); - it("renders the empty state when content is empty", () => { + // #1860: this panel only mounts once a result exists, so an empty result must + // read as "the call completed and returned nothing" — never as the pre-run + // "No results yet" placeholder, which denies a call the user watched succeed. + it("names the outcome when a completed result has no content", () => { renderWithMantine( {}} />, ); - expect(screen.getByText("No results yet")).toBeInTheDocument(); + expect(screen.getByText("Empty result")).toBeInTheDocument(); + expect( + screen.getByText( + "The tool call completed successfully and returned no content.", + ), + ).toBeInTheDocument(); + expect(screen.queryByText("No results yet")).not.toBeInTheDocument(); }); it("groups resource_link blocks in a scrollable Resource Links box", async () => { @@ -141,7 +150,7 @@ describe("ToolResultPanel", () => { onClear={() => {}} />, ); - expect(screen.queryByText("No results yet")).not.toBeInTheDocument(); + expect(screen.queryByText("Empty result")).not.toBeInTheDocument(); expect( screen.getByRole("heading", { name: "Structured Output" }), ).toBeInTheDocument(); diff --git a/clients/web/src/components/groups/ToolResultPanel/ToolResultPanel.tsx b/clients/web/src/components/groups/ToolResultPanel/ToolResultPanel.tsx index 33ca7761..772965dc 100644 --- a/clients/web/src/components/groups/ToolResultPanel/ToolResultPanel.tsx +++ b/clients/web/src/components/groups/ToolResultPanel/ToolResultPanel.tsx @@ -5,7 +5,6 @@ import { Paper, ScrollArea, Stack, - Text, Title, } from "@mantine/core"; import type { @@ -176,6 +175,16 @@ const ErrorAlert = Alert.withProps({ title: "Tool Error", }); +// A completed call whose result renders nothing. `content: []` with no +// `structuredContent` is a legal CallToolResult, and this panel is only mounted +// once a result exists — so saying "no results yet" here would deny a call the +// user just watched succeed (#1860). Name the outcome instead. +const EmptyResultAlert = Alert.withProps({ + color: "gray", + variant: "light", + title: "Empty result", +}); + function ResourceLinksGroup({ links, onReadResource, @@ -277,7 +286,9 @@ export function ToolResultPanel({ ) : result.content.length === 0 && !structuredNode ? ( - No results yet + + The tool call completed successfully and returned no content. + ) : hasLinks ? ( diff --git a/test-servers/configs/mrtr-showcase-http.json b/test-servers/configs/mrtr-showcase-http.json index 026d43c0..84e309fd 100644 --- a/test-servers/configs/mrtr-showcase-http.json +++ b/test-servers/configs/mrtr-showcase-http.json @@ -10,6 +10,7 @@ { "preset": "mrtr_sample" }, { "preset": "mrtr_roots" }, { "preset": "mrtr_edge" }, + { "preset": "mrtr_empty" }, { "preset": "mrtr_loop" } ], "transport": { diff --git a/test-servers/src/preset-registry.ts b/test-servers/src/preset-registry.ts index d78998ec..be840183 100644 --- a/test-servers/src/preset-registry.ts +++ b/test-servers/src/preset-registry.ts @@ -29,6 +29,7 @@ import { createMrtrSamplingTool, createMrtrLoopTool, createMrtrEdgeCaseTool, + createMrtrEmptyResultTool, createCollectUrlElicitationTool, createUrlElicitationFormTool, createSendNotificationTool, @@ -151,6 +152,8 @@ function resolveToolPreset( return createMrtrLoopTool(); case "mrtr_edge": return createMrtrEdgeCaseTool(); + case "mrtr_empty": + return createMrtrEmptyResultTool(); case "collect_url_elicitation": return createCollectUrlElicitationTool(); case "url_elicitation_form": diff --git a/test-servers/src/test-server-fixtures.ts b/test-servers/src/test-server-fixtures.ts index 070b5750..469100ea 100644 --- a/test-servers/src/test-server-fixtures.ts +++ b/test-servers/src/test-server-fixtures.ts @@ -755,6 +755,50 @@ export function createMrtrEdgeCaseTool(): ToolDefinition { }; } +/** + * An MRTR tool whose FINAL (complete) result carries an empty `content` array + * and no `structuredContent` — a legal `CallToolResult` that renders nothing. + * + * This is the shape behind #1860: the sequence completes (the Protocol tab shows + * the conversation COMPLETE), a real result is stored, and the Results panel had + * no way to say so — it fell through to the same "No results yet" placeholder it + * shows before anything has run, so a successful call read as a call that never + * happened. Kept as an MRTR tool rather than a plain one because the multi-round + * case is where the ambiguity actually misleads: the user has answered prompts + * and watched rounds complete, so "no results yet" is flatly contradicted by + * what they just did. + */ +export function createMrtrEmptyResultTool(): ToolDefinition { + return { + name: "mrtr_empty", + description: + "MRTR tool that completes with an empty result (no content, no structuredContent).", + inputSchema: {}, + handler: async ( + _params: Record, + _context?: TestServerContext, + extra?: HandlerExtra, + ) => { + if (extra?.inputResponses?.ack === undefined) { + return inputRequired({ + inputRequests: { + ack: inputRequired.elicit({ + message: "Acknowledge to finish (the result will be empty)", + requestedSchema: { + type: "object", + properties: { ack: { type: "boolean", title: "Acknowledge" } }, + required: ["ack"], + }, + }), + }, + requestState: `mrtr-empty:${++mrtrMintCount}`, + }); + } + return { content: [] } satisfies CallToolResult; + }, + }; +} + /** * Create a "url_elicitation_form" tool that spins up a simple HTTP server on a dynamic * port with a form page, sends that URL via URL elicitation, and on form submit collects From f5757c0f950288880f6f75bfbf4846006eabcbde Mon Sep 17 00:00:00 2001 From: cliffhall Date: Sun, 16 Aug 2026 18:41:36 -0400 Subject: [PATCH 02/13] test(integration): drive the mrtr_empty reproduction through the showcase config MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Copilot review: the unit test covered the panel's rendering, but nothing executed `mrtr_empty`, so the handler, its preset-registry branch, and the showcase config entry could regress with CI green. Adds a live modern-era test that loads `mrtr-showcase-http.json` through `loadConfig`/`resolveConfig` — the same artifacts the README's manual reproduction uses, so the config entry and the registry branch are covered rather than just the fixture factory — answers the embedded elicitation, and asserts the terminal result really is an empty `CallToolResult`: that is the input the Tools panel must render as "completed, returned nothing". Signed-off-by: cliffhall --- .../mcp/inspectorClient-modern-era.test.ts | 48 +++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/clients/web/src/test/integration/mcp/inspectorClient-modern-era.test.ts b/clients/web/src/test/integration/mcp/inspectorClient-modern-era.test.ts index 6251340d..cb346ec0 100644 --- a/clients/web/src/test/integration/mcp/inspectorClient-modern-era.test.ts +++ b/clients/web/src/test/integration/mcp/inspectorClient-modern-era.test.ts @@ -18,6 +18,8 @@ import { createMrtrSamplingTool, createMrtrLoopTool, createMrtrEdgeCaseTool, + loadConfig, + resolveConfig, } from "@modelcontextprotocol/inspector-test-server"; import type { ServerConfig } from "@modelcontextprotocol/inspector-test-server"; import type { @@ -26,6 +28,13 @@ import type { } from "@modelcontextprotocol/client"; import { LOG_LEVEL_META_KEY } from "@modelcontextprotocol/client"; import type { MessageEntry } from "@inspector/core/mcp/types.js"; +import { dirname, join } from "node:path"; +import { fileURLToPath } from "node:url"; + +// clients/web/src/test/integration/mcp → repo root is six levels up. Derived +// from the module URL rather than `process.cwd()` so the showcase config +// resolves the same however vitest is invoked. +const repoRoot = join(dirname(fileURLToPath(import.meta.url)), "../".repeat(6)); /** * Live coverage of the modern (2026-07-28) connection path (#1700). The bundled @@ -389,6 +398,45 @@ describe("modern-era negotiation (2026-07-28)", () => { expect(Object.keys(retry2.inputResponses ?? {})).toHaveLength(0); }); + // #1860. Drives the reproduction the README documents, through the SAME + // artifacts a human would use — the showcase config file, resolved through the + // preset registry — so the config entry, the `mrtr_empty` registry branch, and + // the handler are all covered rather than just the fixture factory. The + // assertion is that a completed MRTR call really does hand back an empty + // `CallToolResult`: that is the input the Tools panel has to render as + // "completed, returned nothing" instead of its pre-run placeholder. + it("completes an MRTR sequence with an empty result (mrtr_empty, via the showcase config)", async () => { + const config = loadConfig( + join(repoRoot, "test-servers/configs/mrtr-showcase-http.json"), + ); + expect(config.tools).toContainEqual({ preset: "mrtr_empty" }); + + const started = createTestServerHttp(resolveConfig(config)); + await started.start(); + server = started; + const connected = await connectWithEra(started.url, "modern"); + + let pausedAtPendingUi = false; + connected.addEventListener("newPendingElicitation", (event) => { + pausedAtPendingUi = true; + void event.detail.respond({ + action: "accept", + content: { ack: true }, + }); + }); + + const { tools } = await connected.listTools(); + const tool = tools.find((t) => t.name === "mrtr_empty"); + expect(tool).toBeDefined(); + + const result = await connected.callTool(tool!, {}); + expect(result.success).toBe(true); + expect(pausedAtPendingUi).toBe(true); + expect(result.result!.content).toEqual([]); + expect(result.result!.structuredContent).toBeUndefined(); + expect(result.result!.isError).toBeFalsy(); + }); + it("cancels an in-flight MRTR call while its embedded request is pending", async () => { const started = await startMrtrServer(createMrtrTool()); const connected = await connectWithEra(started.url, "modern"); From 07a7b0beb5a1875e90e607bba53a9c5837bebb5e Mon Sep 17 00:00:00 2001 From: cliffhall Date: Sun, 16 Aug 2026 20:07:16 -0400 Subject: [PATCH 03/13] fix(web): accept CRLF-delimited SSE frames on the servers events stream MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `/api/servers/events` reader in `useServers` searched for `\n\n` and consumed exactly two characters as the separator. SSE permits CRLF line endings, so a `event: change\r\n\r\n` frame stayed buffered forever: `isSseDataFrame()` was never reached and no background refresh fired, leaving the server list stale until a manual refresh or an LF-delimited event arrived. Frame boundaries are now located with a CRLF-aware matcher that reports the separator's real length, and frame lines split on either terminator. A bare CR is deliberately not accepted as a terminator: it cannot be distinguished from the first half of a CRLF straddling a chunk boundary, so a buffer ending in `…\r\n\r` would read as a completed frame and the `\n` arriving next would open a bogus one. Two regressions cover it, both verified failing against the old parser. Each uses a referentially stable `fetchFn` and holds the frame behind a promise released after the disk write — with an inline `fetchFn` the hook re-subscribes every render and that re-subscription's own mount refresh picks the edit up regardless, so the test would pass unfixed. Closes #2006 Signed-off-by: cliffhall --- .../src/test/core/react/useServers.test.tsx | 146 ++++++++++++++++++ core/react/useServers.ts | 44 +++++- 2 files changed, 183 insertions(+), 7 deletions(-) diff --git a/clients/web/src/test/core/react/useServers.test.tsx b/clients/web/src/test/core/react/useServers.test.tsx index bb95402d..e7c2fbf3 100644 --- a/clients/web/src/test/core/react/useServers.test.tsx +++ b/clients/web/src/test/core/react/useServers.test.tsx @@ -969,6 +969,152 @@ describe("useServers", () => { ); }); + it("parses CRLF-delimited SSE frames as change notifications (#2006)", async () => { + // SSE permits CRLF line endings, and this hook is a hand-rolled parser + // (EventSource can't send `x-mcp-remote-auth`). Matching only `\n\n` left + // a CRLF frame buffered forever, so an external mcp.json edit never + // reached the list. + writeFileSync( + h.configPath, + JSON.stringify({ + mcpServers: { seed: { type: "stdio", command: "s" } }, + }), + ); + + // The frame is held back until the test has mutated the file, so the + // resulting list can only come from a refresh this frame triggered. + let releaseFrame: (() => void) | undefined; + const framed = new Promise((r) => { + releaseFrame = r; + }); + let reads = 0; + const encoder = new TextEncoder(); + // Referentially stable: an inline arrow re-subscribes on every render, and + // the re-subscription's own mount refresh would pick the edit up on its + // own — passing whether or not the frame was ever parsed. + const fetchFn: typeof fetch = async (input, init) => { + const url = input instanceof Request ? input.url : String(input); + if (url.endsWith("/api/servers/events")) { + const body = { + getReader: () => ({ + read: async () => { + reads += 1; + if (reads === 1) { + await framed; + return { + done: false, + value: encoder.encode("event: change\r\n\r\n"), + }; + } + return { done: true, value: undefined }; + }, + }), + }; + return { ok: true, body } as unknown as Response; + } + return h.fetchFn(url, init); + }; + const { result } = renderHook(() => + useServers({ baseUrl: "http://test.local", fetchFn }), + ); + await waitFor(() => expect(result.current.loading).toBe(false)); + expect(result.current.servers.map((s) => s.id)).toEqual(["seed"]); + + writeFileSync( + h.configPath, + JSON.stringify({ + mcpServers: { aftercrlf: { type: "stdio", command: "x" } }, + }), + ); + releaseFrame?.(); + + await waitFor( + () => { + expect(result.current.servers.map((s) => s.id)).toEqual(["aftercrlf"]); + }, + { timeout: 3000 }, + ); + }); + + it("does not split a CRLF frame separator straddling two chunks (#2006)", async () => { + // The bare-CR terminator is deliberately not accepted: a chunk ending in + // `…\r\n\r` would otherwise read as a completed frame, and the `\n` + // arriving next would start a bogus one. Deliver exactly that split and + // assert the frame is recognized once — only after both halves land. + writeFileSync( + h.configPath, + JSON.stringify({ + mcpServers: { seed: { type: "stdio", command: "s" } }, + }), + ); + + let releaseFirstHalf: (() => void) | undefined; + const firstHalf = new Promise((r) => { + releaseFirstHalf = r; + }); + let releaseSecondHalf: (() => void) | undefined; + const secondHalf = new Promise((r) => { + releaseSecondHalf = r; + }); + let listGets = 0; + let reads = 0; + const encoder = new TextEncoder(); + const fetchFn: typeof fetch = async (input, init) => { + const url = input instanceof Request ? input.url : String(input); + if (url.endsWith("/api/servers/events")) { + const body = { + getReader: () => ({ + read: async () => { + reads += 1; + if (reads === 1) { + await firstHalf; + return { + done: false, + value: encoder.encode("event: change\r\n\r"), + }; + } + if (reads === 2) { + await secondHalf; + return { done: false, value: encoder.encode("\n") }; + } + return { done: true, value: undefined }; + }, + }), + }; + return { ok: true, body } as unknown as Response; + } + if (url.endsWith("/api/servers")) listGets += 1; + return h.fetchFn(url, init); + }; + const { result } = renderHook(() => + useServers({ baseUrl: "http://test.local", fetchFn }), + ); + await waitFor(() => expect(result.current.loading).toBe(false)); + expect(listGets).toBe(1); + + // Half a separator is not a frame: no refresh yet. + releaseFirstHalf?.(); + await waitFor(() => expect(reads).toBeGreaterThanOrEqual(2)); + await new Promise((r) => setTimeout(r, 100)); + expect(listGets).toBe(1); + + writeFileSync( + h.configPath, + JSON.stringify({ + mcpServers: { aftersplit: { type: "stdio", command: "x" } }, + }), + ); + releaseSecondHalf?.(); + + await waitFor( + () => { + expect(result.current.servers.map((s) => s.id)).toEqual(["aftersplit"]); + }, + { timeout: 3000 }, + ); + expect(listGets).toBe(2); + }); + it("ignores the backend's inert priming comment frame (#1858)", async () => { // The backend opens every SSE stream with a `:` comment so a streaming // fetch() resolves on Firefox at all. That frame carries no event/data diff --git a/core/react/useServers.ts b/core/react/useServers.ts index 3a2a5002..c9713344 100644 --- a/core/react/useServers.ts +++ b/core/react/useServers.ts @@ -90,10 +90,38 @@ async function readErrorMessage(res: Response): Promise { */ function isSseDataFrame(frame: string): boolean { return frame - .split("\n") + .split(SSE_LINE_SEPARATOR) .some((line) => line.startsWith("event:") || line.startsWith("data:")); } +/** + * SSE line terminators. The spec permits CRLF, LF, and a bare CR; we accept + * the first two. A bare CR is deliberately excluded because it can't be told + * apart from the first half of a CRLF that straddles a chunk boundary — a + * buffer ending in `…\r\n\r` would look like a completed frame and the `\n` + * arriving in the next chunk would be mis-read as the start of another. No + * SSE producer in the wild emits bare-CR frames, so the ambiguity buys + * nothing. + * + * A frame ends at a blank line, i.e. two terminators back to back. + */ +const SSE_LINE_SEPARATOR = /\r\n|\n/; +const SSE_FRAME_SEPARATOR = /(?:\r\n|\n){2}/; + +/** + * Locates the next frame boundary in `buffer`. Returns the frame's end offset + * and the length of the blank-line separator that terminated it (2–4 chars, + * depending on which terminators the server used), or `null` when the buffer + * holds no complete frame yet. + */ +function findSseFrameEnd( + buffer: string, +): { end: number; separatorLength: number } | null { + const match = SSE_FRAME_SEPARATOR.exec(buffer); + if (!match) return null; + return { end: match.index, separatorLength: match[0].length }; +} + export function useServers(opts: UseServersOptions): UseServersResult { const { baseUrl, authToken, fetchFn } = opts; const doFetch = fetchFn ?? globalThis.fetch; @@ -163,7 +191,9 @@ export function useServers(opts: UseServersOptions): UseServersResult { const reader = res.body.getReader(); const decoder = new TextDecoder(); let buffer = ""; - // SSE frames are separated by a blank line (`\n\n`). We don't parse + // SSE frames are separated by a blank line — LF- or CRLF-terminated, + // both of which the spec permits and `findSseFrameEnd` accepts (#2006). + // We don't parse // the event type or data — `refreshInternal()` re-fetches the // canonical state regardless — so we just count frames and fire a // single background refresh per decode chunk. Two `change` @@ -183,12 +213,12 @@ export function useServers(opts: UseServersOptions): UseServersResult { if (done) break; buffer += decoder.decode(value, { stream: true }); let sawFrame = false; - let frameEnd = buffer.indexOf("\n\n"); - while (frameEnd !== -1) { - const frame = buffer.slice(0, frameEnd); - buffer = buffer.slice(frameEnd + 2); + let boundary = findSseFrameEnd(buffer); + while (boundary !== null) { + const frame = buffer.slice(0, boundary.end); + buffer = buffer.slice(boundary.end + boundary.separatorLength); if (isSseDataFrame(frame)) sawFrame = true; - frameEnd = buffer.indexOf("\n\n"); + boundary = findSseFrameEnd(buffer); } if (sawFrame) void refreshInternal(true); } From 364d8c9dbc01de49b2c7d28366332e00dd514d23 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Sun, 16 Aug 2026 20:23:25 -0400 Subject: [PATCH 04/13] feat(auth): per-server authorization and token URL overrides MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds two optional per-server fields — `oauth.authorizationUrl` and `oauth.tokenUrl` — that override the endpoints authorization-server metadata discovery resolved, so a server can be pointed at a development or staging authorization server without changing what it advertises. SDK v2 routes both endpoints through the discovered metadata document and neither through OAuthClientProvider, so the override is applied by wrapping the client's base fetch and patching the metadata document in flight (core/auth/endpointOverrides.ts). Wrapping the base fetch — rather than only the auth fetch — also covers the discovery the SDK runs from inside the transport on the 401/refresh path. The fields round-trip through mcp.json, the /api/servers routes, the runner (CLI/TUI), and the Server Settings → Authorization panel, where a value that is not an absolute http(s) URL is flagged inline; at runtime such a value is dropped with a single warning rather than failing the connection. Signed-off-by: cliffhall --- AGENTS.md | 8 +- README.md | 2 +- clients/web/src/App.tsx | 6 + .../ServerSettingsForm.test.tsx | 76 +++++ .../ServerSettingsForm/ServerSettingsForm.tsx | 52 ++++ .../ServerSettingsModal.test.tsx | 46 +++ .../ServerSettingsModal.tsx | 5 + .../test/core/auth/endpointOverrides.test.ts | 276 ++++++++++++++++++ .../web/src/test/core/client/runner.test.ts | 40 +++ ...torClient-oauth-endpoint-overrides.test.ts | 143 +++++++++ .../src/test/core/mcp/oauthManager.test.ts | 24 ++ .../web/src/test/core/mcp/serverList.test.ts | 61 ++++ .../mcp/remote/servers-route.test.ts | 69 +++++ core/auth/endpointOverrides.ts | 246 ++++++++++++++++ core/auth/index.ts | 10 + core/client/runner.ts | 10 +- core/mcp/inspectorClient.ts | 14 + core/mcp/oauthManager.ts | 18 ++ core/mcp/remote/node/server.ts | 31 +- core/mcp/serverList.ts | 36 +++ core/mcp/types.ts | 39 +++ docs/mcp-server-configuration.md | 6 +- 22 files changed, 1212 insertions(+), 6 deletions(-) create mode 100644 clients/web/src/test/core/auth/endpointOverrides.test.ts create mode 100644 clients/web/src/test/core/mcp/inspectorClient-oauth-endpoint-overrides.test.ts create mode 100644 core/auth/endpointOverrides.ts diff --git a/AGENTS.md b/AGENTS.md index d07e84f4..3deca03b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -45,7 +45,13 @@ v2/main/ │ │ # providers.redirectToAuthorization, the only │ │ # seam that sees the SDK-built authorize URL; │ │ # authorization request only, never the token -│ │ # request — #2018) +│ │ # request — #2018; +│ │ # endpointOverrides.ts per-server +│ │ # authorization/token URL overrides — a fetch +│ │ # wrapper that rewrites the discovered AS +│ │ # metadata document, the one seam SDK v2 routes +│ │ # BOTH endpoints through (neither reaches the +│ │ # OAuthClientProvider) — #1906) │ │ ├── browser/ # Browser-side OAuth (sessionStorage, BrowserNavigation) │ │ ├── node/ # Node-side OAuth (NodeOAuthStorage, OAuthCallbackServer, │ │ │ # runner-interactive-oauth loopback callback flow) diff --git a/README.md b/README.md index cb57dd35..a9fab0c1 100644 --- a/README.md +++ b/README.md @@ -30,7 +30,7 @@ inspector/ │ ├── tui/ # TUI client (Ink + React, tsup bundle) │ └── launcher/ # Shared launcher — provides the `mcp-inspector` bin, dispatches to web/cli/tui ├── core/ # Shared code consumed via the `@inspector/core` alias (no package.json) -│ ├── auth/ # OAuth: providers, discovery, storage, mid-session recovery (browser/node/remote backends) +│ ├── auth/ # OAuth: providers, discovery, storage, endpoint overrides, mid-session recovery (browser/node/remote backends) │ ├── client/ # Install-level client config (`client.json`): browser-safe parse/validate + Node load/save, remote backend, secrets │ ├── json/ # JSON + parameter/argument conversion utilities, and the nullable-union │ │ # schema collapse shared by the web and TUI form builders diff --git a/clients/web/src/App.tsx b/clients/web/src/App.tsx index 72d8bd1f..5837f27f 100644 --- a/clients/web/src/App.tsx +++ b/clients/web/src/App.tsx @@ -64,6 +64,7 @@ import { ResourceSubscriptionsState } from "@inspector/core/mcp/state/resourceSu import { cleanRoots, oauthAuthorizationParamsFromSettings, + oauthEndpointOverridesFromSettings, serializeMcpConfig, } from "@inspector/core/mcp/serverList.js"; import type { ClientConfig } from "@inspector/core/client/types.js"; @@ -2315,12 +2316,16 @@ function App() { const serverAuthorizationParams = savedSettings ? oauthAuthorizationParamsFromSettings(savedSettings) : undefined; + const serverEndpointOverrides = savedSettings + ? oauthEndpointOverridesFromSettings(savedSettings) + : undefined; const oauthFromServer = savedSettings && (savedSettings.oauthClientId || savedSettings.oauthClientSecret || savedSettings.oauthScopes || serverAuthorizationParams || + serverEndpointOverrides || savedSettings.enterpriseManaged) ? { ...(savedSettings.oauthClientId && { @@ -2335,6 +2340,7 @@ function App() { ...(serverAuthorizationParams && { authorizationParams: serverAuthorizationParams, }), + ...serverEndpointOverrides, ...(savedSettings.enterpriseManaged && { enterpriseManaged: true, }), diff --git a/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.test.tsx b/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.test.tsx index d238228a..ac26d2eb 100644 --- a/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.test.tsx +++ b/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.test.tsx @@ -701,6 +701,8 @@ describe("ServerSettingsForm", () => { clientSecret: "", scopes: "", authorizationParams: [], + authorizationUrl: "", + tokenUrl: "", enterpriseManaged: false, }); }); @@ -966,6 +968,76 @@ describe("ServerSettingsForm", () => { expect(screen.getByText(/state, scope are set/)).toBeInTheDocument(); }); + // #1906 — the endpoint overrides ride the same `onOAuthChange` callback as + // the rest of the Authorization section. + it("invokes onOAuthChange when an endpoint override is typed", async () => { + const user = userEvent.setup(); + const onOAuthChange = vi.fn(); + renderWithMantine( + , + ); + await user.type( + screen.getByRole("textbox", { name: /Authorization URL override/i }), + "h", + ); + expect(onOAuthChange).toHaveBeenCalledWith( + expect.objectContaining({ authorizationUrl: "h" }), + ); + + onOAuthChange.mockClear(); + await user.type( + screen.getByRole("textbox", { name: /Token URL override/i }), + "h", + ); + expect(onOAuthChange).toHaveBeenCalledWith( + expect.objectContaining({ tokenUrl: "h" }), + ); + }); + + it("flags an endpoint override that is not an absolute http(s) URL", () => { + renderWithMantine( + , + ); + expect( + screen.getByText('"/authorize" is not an absolute URL.'), + ).toBeInTheDocument(); + expect(screen.queryByText(/is not an http\(s\) URL/)).toBeNull(); + }); + + it("clears an endpoint override through its clear button", async () => { + const user = userEvent.setup(); + const onOAuthChange = vi.fn(); + renderWithMantine( + , + ); + const clearButtons = screen.getAllByRole("button", { name: /clear/i }); + await user.click(clearButtons[clearButtons.length - 1]); + expect(onOAuthChange).toHaveBeenCalledWith( + expect.objectContaining({ tokenUrl: "" }), + ); + }); + it("invokes onOAuthChange with the chosen insufficient-scope policy (SEP-2350)", async () => { const user = userEvent.setup(); const onOAuthChange = vi.fn(); @@ -1007,6 +1079,8 @@ describe("ServerSettingsForm", () => { clientSecret: "", scopes: "", authorizationParams: [], + authorizationUrl: "", + tokenUrl: "", enterpriseManaged: true, }); }); @@ -1112,6 +1186,8 @@ describe("ServerSettingsForm", () => { clientSecret: "z", scopes: "", authorizationParams: [], + authorizationUrl: "", + tokenUrl: "", enterpriseManaged: false, }); }); diff --git a/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.tsx b/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.tsx index 276c9cf9..887555c3 100644 --- a/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.tsx +++ b/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.tsx @@ -32,6 +32,7 @@ import { authorizationParamKeyError, isReservedAuthorizationParam, } from "@inspector/core/auth/authorizationParams.js"; +import { oauthEndpointUrlError } from "@inspector/core/auth/endpointOverrides.js"; import { ADVERTISABLE_EXTENSIONS } from "@inspector/core/mcp/extensions.js"; import type { Root } from "@modelcontextprotocol/client"; @@ -478,6 +479,8 @@ export function ServerSettingsForm({ clientSecret: settings.oauthClientSecret ?? "", scopes: settings.oauthScopes ?? "", authorizationParams, + authorizationUrl: settings.oauthAuthorizationUrl ?? "", + tokenUrl: settings.oauthTokenUrl ?? "", enterpriseManaged: settings.enterpriseManaged ?? false, onInsufficientScope: settings.oauthOnInsufficientScope, }; @@ -863,6 +866,55 @@ export function ServerSettingsForm({ + Add Parameter + + onOAuthChange({ + ...currentOAuth(), + authorizationUrl: e.currentTarget.value, + }) + } + rightSection={ + settings.oauthAuthorizationUrl ? ( + + onOAuthChange({ + ...currentOAuth(), + authorizationUrl: "", + }) + } + /> + ) : null + } + /> + + onOAuthChange({ + ...currentOAuth(), + tokenUrl: e.currentTarget.value, + }) + } + rightSection={ + settings.oauthTokenUrl ? ( + + onOAuthChange({ ...currentOAuth(), tokenUrl: "" }) + } + /> + ) : null + } + />