From 1a2ed756bd31a41bf50913549f056c3c1a994177 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Fri, 5 Jun 2026 11:29:05 -0400 Subject: [PATCH 1/7] feat(web): two-step URL elicitation completion (#1415) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace accept-on-open with an explicit completion step for URL elicitations. Opening the URL now only transitions the panel to its waiting state; the elicitation resolves with `accept` only when the user clicks the revealed "I've completed it" action, or with `cancel`. - ElicitationUrlPanel: add `onComplete`, reveal the completion button and relabel open→"Reopen in Browser" while `isWaiting`. - PendingClientRequestModal: wire local `isWaiting`; open no longer sends a response. Removed the accept-on-open comment that pointed here. - Tests cover open→waiting→complete and open→cancel, and assert opening alone does not resolve the elicitation. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../ElicitationUrlPanel.stories.tsx | 1 + .../ElicitationUrlPanel.test.tsx | 40 +++++++++++++++++-- .../ElicitationUrlPanel.tsx | 20 +++++++++- .../PendingClientRequestModal.test.tsx | 29 +++++++++++++- .../PendingClientRequestModal.tsx | 19 ++++----- 5 files changed, 92 insertions(+), 17 deletions(-) diff --git a/clients/web/src/components/groups/ElicitationUrlPanel/ElicitationUrlPanel.stories.tsx b/clients/web/src/components/groups/ElicitationUrlPanel/ElicitationUrlPanel.stories.tsx index d70bc3b08..935f9e487 100644 --- a/clients/web/src/components/groups/ElicitationUrlPanel/ElicitationUrlPanel.stories.tsx +++ b/clients/web/src/components/groups/ElicitationUrlPanel/ElicitationUrlPanel.stories.tsx @@ -8,6 +8,7 @@ const meta: Meta = { args: { onCopyUrl: fn(), onOpenInBrowser: fn(), + onComplete: fn(), onCancel: fn(), message: "Please authenticate with the external service.", requestId: "elicit-abc-123", diff --git a/clients/web/src/components/groups/ElicitationUrlPanel/ElicitationUrlPanel.test.tsx b/clients/web/src/components/groups/ElicitationUrlPanel/ElicitationUrlPanel.test.tsx index 37007c3c7..82eea6930 100644 --- a/clients/web/src/components/groups/ElicitationUrlPanel/ElicitationUrlPanel.test.tsx +++ b/clients/web/src/components/groups/ElicitationUrlPanel/ElicitationUrlPanel.test.tsx @@ -10,6 +10,7 @@ const baseProps = { isWaiting: false, onCopyUrl: vi.fn(), onOpenInBrowser: vi.fn(), + onComplete: vi.fn(), onCancel: vi.fn(), }; @@ -33,16 +34,29 @@ describe("ElicitationUrlPanel", () => { ).toBeInTheDocument(); }); - it("does not render waiting indicator when isWaiting is false", () => { + it("does not render the waiting indicator or complete action when not waiting", () => { renderWithMantine(); expect( - screen.queryByText("Waiting for completion..."), + screen.queryByText(/Waiting for completion/), ).not.toBeInTheDocument(); + expect( + screen.queryByRole("button", { name: "I've completed it" }), + ).not.toBeInTheDocument(); + expect( + screen.getByRole("button", { name: "Open in Browser" }), + ).toBeInTheDocument(); }); - it("renders waiting indicator when isWaiting is true", () => { + it("renders the waiting indicator and complete action when waiting", () => { renderWithMantine(); - expect(screen.getByText("Waiting for completion...")).toBeInTheDocument(); + expect(screen.getByText(/Waiting for completion/)).toBeInTheDocument(); + expect( + screen.getByRole("button", { name: "I've completed it" }), + ).toBeInTheDocument(); + // The open action is relabelled so the user can reopen the tab. + expect( + screen.getByRole("button", { name: "Reopen in Browser" }), + ).toBeInTheDocument(); }); it("invokes onCopyUrl when Copy URL is clicked", async () => { @@ -65,6 +79,16 @@ describe("ElicitationUrlPanel", () => { expect(onOpenInBrowser).toHaveBeenCalledTimes(1); }); + it("invokes onComplete when I've completed it is clicked while waiting", async () => { + const user = userEvent.setup(); + const onComplete = vi.fn(); + renderWithMantine( + , + ); + await user.click(screen.getByRole("button", { name: "I've completed it" })); + expect(onComplete).toHaveBeenCalledTimes(1); + }); + it("invokes onCancel when Cancel is clicked", async () => { const user = userEvent.setup(); const onCancel = vi.fn(); @@ -74,4 +98,12 @@ describe("ElicitationUrlPanel", () => { await user.click(screen.getByRole("button", { name: "Cancel" })); expect(onCancel).toHaveBeenCalledTimes(1); }); + + it("disables the completion and cancel actions when busy", () => { + renderWithMantine(); + expect( + screen.getByRole("button", { name: "I've completed it" }), + ).toBeDisabled(); + expect(screen.getByRole("button", { name: "Cancel" })).toBeDisabled(); + }); }); diff --git a/clients/web/src/components/groups/ElicitationUrlPanel/ElicitationUrlPanel.tsx b/clients/web/src/components/groups/ElicitationUrlPanel/ElicitationUrlPanel.tsx index 5e9f57638..bc2734457 100644 --- a/clients/web/src/components/groups/ElicitationUrlPanel/ElicitationUrlPanel.tsx +++ b/clients/web/src/components/groups/ElicitationUrlPanel/ElicitationUrlPanel.tsx @@ -13,9 +13,16 @@ export interface ElicitationUrlPanelProps { message: string; url: string; requestId: string; + /** + * The URL has been opened and we're waiting for the user to confirm they + * finished the external flow. Reveals the "I've completed it" action; opening + * the URL does not resolve the elicitation on its own. + */ isWaiting: boolean; onCopyUrl: () => void; onOpenInBrowser: () => void; + /** User confirms the external flow is done — sends `accept`. */ + onComplete: () => void; onCancel: () => void; /** * A response has been dispatched; lock the responding actions so a second @@ -56,6 +63,7 @@ export function ElicitationUrlPanel({ isWaiting, onCopyUrl, onOpenInBrowser, + onComplete, onCancel, busy = false, }: ElicitationUrlPanelProps) { @@ -70,14 +78,17 @@ export function ElicitationUrlPanel({ Copy URL {isWaiting && ( - Waiting for completion... + + Waiting for completion... Confirm once you've finished in the + browser. + )} {formatRequestId(requestId)} @@ -88,6 +99,11 @@ export function ElicitationUrlPanel({ + {isWaiting && ( + + )} ); diff --git a/clients/web/src/components/groups/PendingClientRequestModal/PendingClientRequestModal.test.tsx b/clients/web/src/components/groups/PendingClientRequestModal/PendingClientRequestModal.test.tsx index 6a09d3c08..6c3cfe52e 100644 --- a/clients/web/src/components/groups/PendingClientRequestModal/PendingClientRequestModal.test.tsx +++ b/clients/web/src/components/groups/PendingClientRequestModal/PendingClientRequestModal.test.tsx @@ -214,18 +214,39 @@ describe("PendingClientRequestModal", () => { }); }); - it("opens the URL and accepts a URL elicitation", async () => { + it("opens the URL into a waiting state without resolving the elicitation", async () => { const user = userEvent.setup(); const openSpy = vi.spyOn(window, "open").mockReturnValue(null); renderWithMantine( , ); + // Before opening there is no completion action. + expect( + screen.queryByRole("button", { name: "I've completed it" }), + ).not.toBeInTheDocument(); await user.click(screen.getByRole("button", { name: "Open in Browser" })); expect(openSpy).toHaveBeenCalledWith( "https://example.com/authorize", "_blank", "noopener,noreferrer", ); + // Opening alone must not resolve the elicitation; it only reveals the + // explicit completion step. + expect(baseProps.onElicitationRespond).not.toHaveBeenCalled(); + expect( + screen.getByRole("button", { name: "I've completed it" }), + ).toBeInTheDocument(); + openSpy.mockRestore(); + }); + + it("accepts a URL elicitation only after the user confirms completion", async () => { + const user = userEvent.setup(); + const openSpy = vi.spyOn(window, "open").mockReturnValue(null); + renderWithMantine( + , + ); + await user.click(screen.getByRole("button", { name: "Open in Browser" })); + await user.click(screen.getByRole("button", { name: "I've completed it" })); expect(baseProps.onElicitationRespond).toHaveBeenCalledWith({ action: "accept", }); @@ -246,14 +267,18 @@ describe("PendingClientRequestModal", () => { expect(writeText).toHaveBeenCalledWith("https://example.com/authorize"); }); - it("cancels a URL elicitation", async () => { + it("cancels a URL elicitation after opening without sending accept", async () => { const user = userEvent.setup(); + const openSpy = vi.spyOn(window, "open").mockReturnValue(null); renderWithMantine( , ); + await user.click(screen.getByRole("button", { name: "Open in Browser" })); await user.click(screen.getByRole("button", { name: "Cancel" })); + expect(baseProps.onElicitationRespond).toHaveBeenCalledTimes(1); expect(baseProps.onElicitationRespond).toHaveBeenCalledWith({ action: "cancel", }); + openSpy.mockRestore(); }); }); diff --git a/clients/web/src/components/groups/PendingClientRequestModal/PendingClientRequestModal.tsx b/clients/web/src/components/groups/PendingClientRequestModal/PendingClientRequestModal.tsx index 8e128672b..3945da5a3 100644 --- a/clients/web/src/components/groups/PendingClientRequestModal/PendingClientRequestModal.tsx +++ b/clients/web/src/components/groups/PendingClientRequestModal/PendingClientRequestModal.tsx @@ -174,25 +174,26 @@ function ElicitationUrlModalBody({ url: string; onRespond: (result: ElicitResult) => void; }) { + const [isWaiting, setIsWaiting] = useState(false); const { responded, once } = useRespondOnce(); return ( { void navigator.clipboard?.writeText(url); }} - onOpenInBrowser={once(() => { + onOpenInBrowser={() => { window.open(url, "_blank", "noopener,noreferrer"); - // Accept-on-open: the inspector can't observe completion of an external - // flow, so opening the URL is treated as acceptance. This is optimistic - // — the user could close the tab without finishing. A proper two-step - // "open, then confirm completion" flow (using ElicitationUrlPanel's - // isWaiting state) is tracked as a follow-up; see #1415. - onRespond({ action: "accept" }); - })} + // Opening the URL only moves the panel into its waiting state — the + // inspector can't observe completion of an external flow, so the + // elicitation resolves only when the user explicitly confirms + // completion (accept) or cancels. + setIsWaiting(true); + }} + onComplete={once(() => onRespond({ action: "accept" }))} onCancel={once(() => onRespond({ action: "cancel" }))} busy={responded} /> From e613abb0c318c264ad5338805d5353ccd93a235a Mon Sep 17 00:00:00 2001 From: cliffhall Date: Fri, 5 Jun 2026 15:16:12 -0400 Subject: [PATCH 2/7] feat(web): handle URL-elicitation-required (-32042) error path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implements the spec's "URL mode with elicitation required error" flow (2025-11-25). When a tools/call returns a `-32042` UrlElicitationRequired error, the client now surfaces the required URL elicitation(s) and retries the call once completed, instead of showing a bare error. Core (InspectorClient.callTool): - Detect `-32042` (typed UrlElicitationRequiredError or generic McpError) via the new `getUrlElicitationsFromError` helper. - Surface each carried elicitation in order through the existing pending- elicitation queue / modal (reusing the two-step "Open → I've completed it" UI), then re-issue the original call once all are accepted. A decline/cancel aborts with a clear message; a bounded retry count guards a server that keeps returning the error. - `ElicitationCreateMessage.completeIfPending()` lets the optional `notifications/elicitation/complete` auto-advance/auto-accept an open URL elicitation; the completion-notification handler now resolves (was: only removed) the pending entry. - Extracted `attemptToolCall` / `dispatchFailedToolCall` from the old callTool body to support the retry loop without duplicating bookkeeping. Web (App): - A non-spec `-32042` with no elicitations (nothing to open) shows a dedicated yellow toast linking to a new UrlElicitationErrorModal with the raw error body, mirroring the output-schema-validation toast. Tests: helper unit tests, completeIfPending unit tests, callTool error-path retry/cancel/no-list/ordinary-error tests (injected fake client), the new modal (test + stories), and an App toast test. Co-Authored-By: Claude Opus 4.8 (1M context) --- clients/web/src/App.test.tsx | 30 +++ clients/web/src/App.tsx | 79 ++++++- .../UrlElicitationErrorModal.stories.tsx | 59 +++++ .../UrlElicitationErrorModal.test.tsx | 63 ++++++ .../UrlElicitationErrorModal.tsx | 64 ++++++ .../core/mcp/elicitationCreateMessage.test.ts | 55 +++++ .../mcp/inspectorClientUrlElicitation.test.ts | 190 ++++++++++++++++ .../src/test/core/mcp/urlElicitation.test.ts | 53 +++++ core/mcp/elicitationCreateMessage.ts | 16 ++ core/mcp/inspectorClient.ts | 203 +++++++++++++++--- core/mcp/urlElicitation.ts | 40 ++++ 11 files changed, 818 insertions(+), 34 deletions(-) create mode 100644 clients/web/src/components/groups/UrlElicitationErrorModal/UrlElicitationErrorModal.stories.tsx create mode 100644 clients/web/src/components/groups/UrlElicitationErrorModal/UrlElicitationErrorModal.test.tsx create mode 100644 clients/web/src/components/groups/UrlElicitationErrorModal/UrlElicitationErrorModal.tsx create mode 100644 clients/web/src/test/core/mcp/elicitationCreateMessage.test.ts create mode 100644 clients/web/src/test/core/mcp/inspectorClientUrlElicitation.test.ts create mode 100644 clients/web/src/test/core/mcp/urlElicitation.test.ts create mode 100644 core/mcp/urlElicitation.ts diff --git a/clients/web/src/App.test.tsx b/clients/web/src/App.test.tsx index 674a106ec..f9b7752d5 100644 --- a/clients/web/src/App.test.tsx +++ b/clients/web/src/App.test.tsx @@ -1,4 +1,5 @@ import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; +import { ErrorCode, McpError } from "@modelcontextprotocol/sdk/types.js"; import { renderWithMantine, screen, @@ -720,6 +721,35 @@ describe("App task wiring", () => { ); }); + it("shows a URL-elicitation toast when a tool call fails with a no-list -32042", async () => { + const user = userEvent.setup(); + renderWithMantine(); + await user.click(screen.getByText("connect")); + await waitFor(() => expect(clientInstances).toHaveLength(1)); + + ( + clientInstances[0] as unknown as { + callTool: ReturnType; + } + ).callTool.mockRejectedValueOnce( + new McpError( + ErrorCode.UrlElicitationRequired, + "This request requires browser-based authorization.", + ), + ); + + await user.click(screen.getByText("call")); + + await waitFor(() => + expect(notificationsMock.show).toHaveBeenCalledWith( + expect.objectContaining({ + title: "URL elicitation required", + color: "yellow", + }), + ), + ); + }); + it("surfaces a refresh failure as a red toast", async () => { vi.mocked(useManagedRequestorTasks).mockReturnValue({ tasks: [], diff --git a/clients/web/src/App.tsx b/clients/web/src/App.tsx index 5dc00dab6..2cbdb508f 100644 --- a/clients/web/src/App.tsx +++ b/clients/web/src/App.tsx @@ -23,6 +23,7 @@ import type { InspectorClientEventMap, JsonValue, } from "@inspector/core/mcp/index.js"; +import { getUrlElicitationsFromError } from "@inspector/core/mcp/urlElicitation.js"; import type { TypedEventGeneric } from "@inspector/core/mcp/typedEventTarget.js"; import type { InspectorServerSettings, @@ -98,6 +99,7 @@ import { import { ServerSettingsModal } from "./components/groups/ServerSettingsModal/ServerSettingsModal"; import { ConnectionInfoModal } from "./components/groups/ConnectionInfoModal/ConnectionInfoModal"; import { OutputValidationModal } from "./components/groups/OutputValidationModal/OutputValidationModal"; +import { UrlElicitationErrorModal } from "./components/groups/UrlElicitationErrorModal/UrlElicitationErrorModal"; import type { OAuthDetails } from "./components/groups/ConnectionInfoContent/ConnectionInfoContent"; import { ServerRemoveConfirmModal } from "./components/groups/ServerRemoveConfirmModal/ServerRemoveConfirmModal"; import { @@ -223,6 +225,46 @@ const OutputValidationToastMessage = ({ ); +// Body of the non-spec URLElicitationRequired toast: the server returned a +// -32042 error with no `elicitations` list, so there's no URL to open. We keep +// the toast short and link to a modal with the raw error body. +const UrlElicitationErrorToastMessage = ({ + onViewDetails, +}: { + onViewDetails: () => void; +}) => ( + + + The server reported a URLElicitationRequired error but listed no required + elicitations, so there's nothing to open. + + + View error details + + +); + +function errorMessage(err: unknown): string { + return err instanceof Error ? err.message : String(err); +} + +// Pretty-print a thrown error for the URL-elicitation details modal: an McpError +// carries a `code`/`data` worth showing alongside the message, so include them +// when present; otherwise fall back to the plain message. +function formatErrorDetails(err: unknown): string { + if (err && typeof err === "object") { + const e = err as { code?: unknown; message?: unknown; data?: unknown }; + if (e.code !== undefined || e.data !== undefined) { + return JSON.stringify( + { code: e.code, message: e.message, data: e.data }, + null, + 2, + ); + } + } + return errorMessage(err); +} + // How long a progress toast lingers after its last tick. Each new tick on the // same progress stream resets this window (via `notifications.update`), so a // steady stream keeps one toast alive; the toast clears a few seconds after @@ -339,6 +381,12 @@ function App() { toolName: string; message: string; } | null>(null); + // Raw body for the non-spec URLElicitationRequired (-32042, no elicitations) + // modal opened from its warning toast. + const [urlElicitationErrorDetails, setUrlElicitationErrorDetails] = useState<{ + toolName: string; + details: string; + } | null>(null); // The active connection target. `null` between sessions; set as soon as // the user toggles a server card on. Drives state-manager lifetime. @@ -1258,9 +1306,32 @@ function App() { error: invocation.error, }); } catch (err) { + // A URLElicitationRequired (-32042) error that reaches here carried no + // elicitations (a non-spec response — the with-list case is handled and + // retried inside callTool). There's no URL to open, so surface a short + // toast that links to the raw error rather than a bare error panel. + const urlElicitations = getUrlElicitationsFromError(err); + if (urlElicitations !== null && urlElicitations.length === 0) { + const details = { + toolName: name, + details: formatErrorDetails(err), + }; + setToolCallState({ status: "error", error: errorMessage(err) }); + notifications.show({ + autoClose: false, + title: "URL elicitation required", + color: "yellow", + message: ( + setUrlElicitationErrorDetails(details)} + /> + ), + }); + return; + } setToolCallState({ status: "error", - error: err instanceof Error ? err.message : String(err), + error: errorMessage(err), }); } }, @@ -1953,6 +2024,12 @@ function App() { message={outputValidationDetails?.message} onClose={() => setOutputValidationDetails(null)} /> + setUrlElicitationErrorDetails(null)} + /> + + + + + ); +} + +const meta: Meta = { + title: "Groups/UrlElicitationErrorModal", + component: UrlElicitationErrorModal, + parameters: { layout: "fullscreen" }, + render: InteractiveRender, + args: { + opened: true, + onClose: fn(), + toolName: "trigger-url-elicitation", + details: SAMPLE_DETAILS, + }, +}; + +export default meta; +type Story = StoryObj; + +export const Default: Story = { + play: async ({ canvasElement }) => { + // Mantine renders the modal in a portal at document.body. + const body = within(canvasElement.ownerDocument.body); + await body.findByText("URL elicitation required"); + expect(body.getByText(/trigger-url-elicitation/)).toBeInTheDocument(); + const details = body.getByLabelText("Error details") as HTMLTextAreaElement; + expect(details.value).toContain("-32042"); + expect(details.readOnly).toBe(true); + }, +}; + +export const WithoutToolName: Story = { + args: { toolName: undefined }, +}; diff --git a/clients/web/src/components/groups/UrlElicitationErrorModal/UrlElicitationErrorModal.test.tsx b/clients/web/src/components/groups/UrlElicitationErrorModal/UrlElicitationErrorModal.test.tsx new file mode 100644 index 000000000..fe52b5b24 --- /dev/null +++ b/clients/web/src/components/groups/UrlElicitationErrorModal/UrlElicitationErrorModal.test.tsx @@ -0,0 +1,63 @@ +import { describe, it, expect, vi } from "vitest"; +import userEvent from "@testing-library/user-event"; +import { renderWithMantine, screen } from "../../../test/renderWithMantine"; +import { UrlElicitationErrorModal } from "./UrlElicitationErrorModal"; + +const DETAILS = JSON.stringify( + { + code: -32042, + message: "This request requires browser-based authorization.", + data: {}, + }, + null, + 2, +); + +describe("UrlElicitationErrorModal", () => { + it("renders the tool name and the raw error body in a read-only field", () => { + renderWithMantine( + , + ); + expect(screen.getByText("URL elicitation required")).toBeInTheDocument(); + expect(screen.getByText(/trigger-url-elicitation/)).toBeInTheDocument(); + const details = screen.getByLabelText( + "Error details", + ) as HTMLTextAreaElement; + expect(details.value).toBe(DETAILS); + expect(details.readOnly).toBe(true); + }); + + it("falls back to a generic message when no tool name is provided", () => { + renderWithMantine( + , + ); + expect(screen.getByText(/no required elicitations/i)).toBeInTheDocument(); + }); + + it("calls onClose when the close button is clicked", async () => { + const onClose = vi.fn(); + renderWithMantine( + , + ); + await userEvent.click(screen.getByRole("button", { name: "Close" })); + expect(onClose).toHaveBeenCalledTimes(1); + }); + + it("renders nothing visible when closed", () => { + renderWithMantine( + , + ); + expect( + screen.queryByText("URL elicitation required"), + ).not.toBeInTheDocument(); + }); +}); diff --git a/clients/web/src/components/groups/UrlElicitationErrorModal/UrlElicitationErrorModal.tsx b/clients/web/src/components/groups/UrlElicitationErrorModal/UrlElicitationErrorModal.tsx new file mode 100644 index 000000000..5f1421b40 --- /dev/null +++ b/clients/web/src/components/groups/UrlElicitationErrorModal/UrlElicitationErrorModal.tsx @@ -0,0 +1,64 @@ +import { + CloseButton, + Group, + Modal, + Stack, + Text, + Textarea, + Title, +} from "@mantine/core"; + +export interface UrlElicitationErrorModalProps { + opened: boolean; + onClose: () => void; + /** The tool whose call returned the URL-elicitation-required error. */ + toolName?: string; + /** The raw error body (message + data), pretty-printed for inspection. */ + details?: string; +} + +/** + * Surfaces the raw body of a `URLElicitationRequired` (`-32042`) error that + * carried no `elicitations` list — a non-spec server response the inspector + * can't act on (there's no URL to open). The Tools screen shows a short toast; + * this modal, opened from that toast, exposes the full error so a server + * developer can see what the server actually returned. + */ +export function UrlElicitationErrorModal({ + opened, + onClose, + toolName, + details, +}: UrlElicitationErrorModalProps) { + return ( + + + + + URL elicitation required + + + + + {toolName + ? `"${toolName}" returned a URLElicitationRequired (-32042) error with no required elicitations. Per the MCP spec the error must list the URL elicitations to complete before retrying, so the inspector has nothing to open.` + : "The server returned a URLElicitationRequired (-32042) error with no required elicitations. Per the MCP spec the error must list the URL elicitations to complete before retrying, so the inspector has nothing to open."} + +