From 08fc20a22d827bcf8894fe3df5a40f10bde650fa Mon Sep 17 00:00:00 2001 From: cliffhall Date: Sat, 22 Aug 2026 19:05:21 -0400 Subject: [PATCH 01/13] feat(apps): negotiated app-rendered form elicitations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Let a server hand a form `elicitation/create` to an MCP App and return the app's ordinary `ElicitResult`, with the native elicitation form as the fallback on every failure. No second extension, no custom method, and no custom result shape: the only new wire surface is a nested `elicitation` flag on the existing `io.modelcontextprotocol/ui` extension on each side, plus `_meta.ui.resourceUri` on the request. App rendering is selected only when all four gates hold — client `elicitation.form`, client MCP Apps MIME type, the nested `elicitation` setting on BOTH peers, and a valid absolute `ui://` URI on the request. Only the web client advertises the nested client-side setting, and only because it has a sandbox renderer: supplying `InspectorClientOptions.appElicitation` is what opts a client in, so CLI and TUI keep advertising the MIME type without ever claiming they can resolve an elicitation through an app. Routing lives in the one funnel both entry points already use (`enqueuePendingElicitation`), so the inbound handler and the MRTR driver cannot diverge. Ownership is request-scoped — a per-request id keys the renderer, iframe and bridge — so two concurrent elicitations can never resolve through each other's bridges. An explicit `decline`/`cancel` is a completed elicitation and goes back to the server; everything else (absent or malformed metadata, resource/sandbox/bridge failure, an app with no elicitation capability, a timeout, an invalid result or one that fails the requested schema) falls back. The Inspector speaks the ext-apps#733 / SEP-3118 wire protocol but cannot yet consume its helpers — the released `@modelcontextprotocol/ext-apps` (1.7.5) predates that PR. `core/mcp/appElicitation.ts` and `AppRenderer/requestAppElicitation.ts` mirror it exactly and are marked for deletion once a release containing it ships. One consequence needed its own seam: ext-apps 1.7.5 parses the view's `ui/initialize` through a schema that strips `elicitation`, so an app that correctly advertises it would look like one that did not, silently turning every negotiated elicitation into a fallback. `AppRenderer/appCapabilities.ts` records the raw frame instead, and prefers the bridge's own value once it carries the key. `AppRenderer` now takes an `AppRenderSource` union rather than a `Tool`, so the same renderer and bridge factory serve an App tool and an elicitation without either faking the other's shape. Adds the public fixture (`app_choose_option` + a self-contained `ui://demo/choose-option.html` app covering accept/decline/cancel), its two showcase configs, and `smoke:web:elicit`, which drives the negotiated path end to end in headless Chromium and then the same tool against a server that never advertised the capability, asserting the native form takes it. Closes #1854 Signed-off-by: cliffhall --- AGENTS.md | 12 +- README.md | 22 +- clients/web/README.md | 9 + clients/web/src/App.tsx | 74 ++- .../AppElicitationHost.stories.tsx | 104 +++++ .../AppElicitationHost.test.tsx | 411 +++++++++++++++++ .../AppElicitation/AppElicitationHost.tsx | 270 +++++++++++ .../AppRenderer/AppRenderer.stories.tsx | 2 +- .../elements/AppRenderer/AppRenderer.test.tsx | 222 ++++++--- .../elements/AppRenderer/AppRenderer.tsx | 53 ++- .../AppRenderer/appCapabilities.test.ts | 94 ++++ .../elements/AppRenderer/appCapabilities.ts | 67 +++ .../AppRenderer/appRenderSource.test.ts | 86 ++++ .../elements/AppRenderer/appRenderSource.ts | 49 ++ .../createAppBridgeFactory.test.ts | 32 +- .../AppRenderer/createAppBridgeFactory.ts | 50 +- .../AppRenderer/requestAppElicitation.test.ts | 91 ++++ .../AppRenderer/requestAppElicitation.ts | 59 +++ .../screens/AppsScreen/AppsScreen.tsx | 2 +- .../src/lib/appElicitationController.test.ts | 119 +++++ .../web/src/lib/appElicitationController.ts | 99 ++++ .../src/test/core/mcp/appElicitation.test.ts | 274 +++++++++++ .../web/src/test/core/mcp/extensions.test.ts | 38 ++ .../inspectorClient-app-elicitation.test.ts | 431 ++++++++++++++++++ .../integration/mcp/appElicitation.test.ts | 168 +++++++ core/mcp/appElicitation.ts | 238 ++++++++++ core/mcp/extensions.ts | 21 + core/mcp/inspectorClient.ts | 138 +++++- core/mcp/types.ts | 16 + package.json | 3 +- scripts/smoke-web-elicitation.mjs | 318 +++++++++++++ .../configs/app-elicitation-http.json | 13 + .../configs/app-elicitation-native-http.json | 12 + test-servers/src/composable-test-server.ts | 27 ++ test-servers/src/load-config.ts | 4 + test-servers/src/preset-registry.ts | 6 + test-servers/src/resolve-config.ts | 1 + test-servers/src/test-server-fixtures.ts | 176 +++++++ 38 files changed, 3718 insertions(+), 93 deletions(-) create mode 100644 clients/web/src/components/elements/AppElicitation/AppElicitationHost.stories.tsx create mode 100644 clients/web/src/components/elements/AppElicitation/AppElicitationHost.test.tsx create mode 100644 clients/web/src/components/elements/AppElicitation/AppElicitationHost.tsx create mode 100644 clients/web/src/components/elements/AppRenderer/appCapabilities.test.ts create mode 100644 clients/web/src/components/elements/AppRenderer/appCapabilities.ts create mode 100644 clients/web/src/components/elements/AppRenderer/appRenderSource.test.ts create mode 100644 clients/web/src/components/elements/AppRenderer/appRenderSource.ts create mode 100644 clients/web/src/components/elements/AppRenderer/requestAppElicitation.test.ts create mode 100644 clients/web/src/components/elements/AppRenderer/requestAppElicitation.ts create mode 100644 clients/web/src/lib/appElicitationController.test.ts create mode 100644 clients/web/src/lib/appElicitationController.ts create mode 100644 clients/web/src/test/core/mcp/appElicitation.test.ts create mode 100644 clients/web/src/test/core/mcp/inspectorClient-app-elicitation.test.ts create mode 100644 clients/web/src/test/integration/mcp/appElicitation.test.ts create mode 100644 core/mcp/appElicitation.ts create mode 100644 scripts/smoke-web-elicitation.mjs create mode 100644 test-servers/configs/app-elicitation-http.json create mode 100644 test-servers/configs/app-elicitation-native-http.json diff --git a/AGENTS.md b/AGENTS.md index 0771175ec8..20b6a42859 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -112,7 +112,17 @@ v2/main/ │ │ # listSalvage.ts: per-item salvage for list results — │ │ # keeps the valid entries when one is non-conforming │ │ # instead of losing the whole list, and owns the -│ │ # shared isClientDecodeRejection predicate — #1909) +│ │ # shared isClientDecodeRejection predicate — #1909; +│ │ # appElicitation.ts: app-rendered form elicitation +│ │ # (#1854) — the four negotiation gates, the +│ │ # `_meta.ui.resourceUri` reader/validator, and the +│ │ # result validator. Mirrors ext-apps#733 / SEP-3118 +│ │ # because the released ext-apps (1.7.5) predates it; +│ │ # the host-side renderer is supplied by the CLIENT +│ │ # (`InspectorClientOptions.appElicitation`), and +│ │ # supplying one is what advertises the nested +│ │ # `elicitation` setting — so web opts in and +│ │ # cli/tui, which cannot host an App, do not) │ │ ├── import/ # Config import strategies (#1348): client-config parsers │ │ │ # (Claude Desktop/Cursor/Cline/VS Code), registry │ │ │ # server.json parser, strategy registry + well-known diff --git a/README.md b/README.md index 2c420c00a3..8ae6e02c96 100644 --- a/README.md +++ b/README.md @@ -140,6 +140,7 @@ Each config below is a ready-made server for exercising one feature by hand. Loa | Config | Demonstrates | Issue | | ----------------------------------------- | -------------------------------------------------- | ---------------------------------------------------------------------- | | `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) | +| `app-elicitation-http.json` **(legacy era)** | An MCP App rendering a form elicitation | [#1854](https://github.com/modelcontextprotocol/inspector/issues/1854) | | `modern-mrtr-http.json` | A single MRTR round-trip | — | | `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) | @@ -162,6 +163,25 @@ Open the Apps tab, select `mcp_app_demo`, give it a title and click **Open App** For the scripted version of the same flow (`--app-info` probe → deep link → rendered widget), see [Reviewing an MCP App](./docs/mcp-app-review.md). +#### App-rendered form elicitations + +`app-elicitation-http.json` serves `app_choose_option` alongside the `choose_option_app` UI resource (`ui://demo/choose-option.html`). The tool sends a completely ordinary form `elicitation/create` — the only thing added is `_meta.ui.resourceUri` naming that app. Plain streamable-HTTP; connect with the **default (legacy)** protocol era. + +Run `app_choose_option` from the Tools tab. The server's app renders in a modal instead of the built-in elicitation form, and clicking **Option A**, **Decline**, or **Cancel** returns the standard `ElicitResult` straight to the server, which echoes it into the tool result. + +App rendering is selected only when **all four** conditions hold ([#1854](https://github.com/modelcontextprotocol/inspector/issues/1854), per [ext-apps#733](https://github.com/modelcontextprotocol/ext-apps/pull/733) / SEP-3118): + +1. the client advertises `elicitation.form`; +2. the client advertises `extensions["io.modelcontextprotocol/ui"].mimeTypes` including `text/html;profile=mcp-app`; +3. **both** the client and the server advertise the nested `elicitation` setting on that same extension; +4. the request carries a valid absolute `ui://` URI in `_meta.ui.resourceUri`. + +Only the **web** client advertises the nested client-side setting, and only because it has a sandbox renderer to back it; the CLI and TUI advertise the MIME type (they know what an App is) but never claim they can resolve an elicitation through one, so the same server falls back to their native prompts. Turning **Server Settings → Advertised Extensions → MCP Apps UI** off, or turning form elicitation off, removes the claim on web too. + +Everything else falls back to the built-in elicitation form, by design: metadata that is absent or not an absolute `ui://` URI, a resource that fails to load, a sandbox or bridge that fails to initialize, an app that did not advertise `elicitation`, a request that times out, and any result that is not a valid `ElicitResult` for the requested schema. An explicit `decline` or `cancel` is **not** a fallback — it is a completed elicitation and goes back to the server as-is. + +> The Inspector speaks the ext-apps#733 wire protocol but does not yet consume its helpers: the released `@modelcontextprotocol/ext-apps` (1.7.5) predates that PR. `core/mcp/appElicitation.ts` and `clients/web/src/components/elements/AppRenderer/requestAppElicitation.ts` mirror it exactly and are marked for deletion in favour of the package's own exports once a release containing it ships. + #### MRTR `modern-mrtr-http.json` serves the `mrtr_confirm` tool (preset `mrtr_confirm`, `createMrtrTool`) over the modern leg. Its handler returns `inputRequired(...)` embedding a form elicitation, so invoking it produces a real round-trip: `input_required` → the client fulfils the embedded elicitation and retries with a new id → `complete`. @@ -343,7 +363,7 @@ Each client self-validates from its own folder; the root scripts chain them. The | ----------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | `npm run validate` | Runs the three durable guards first — `verify:format-coverage` (every tracked source file is format-gated), `verify:typecheck-coverage` (every one lands in a tsconfig project), `verify:dep-lockstep` (no dependency reaching one `tsc` program from two installs skews across them) — then `test:scripts` (the guards' own parser unit tests), then `validate:core` (the shared `core/` `format:check` + `lint` gate), then per client: `format:check` + `lint` + **`typecheck`** (cli/tui/launcher; web typechecks via `tsc -b` inside its `build`) + `build` + fast unit tests. The quick inner-loop check. | | `npm run coverage` | The **per-file ≥90% gate** (lines/statements/functions/branches) under v8 instrumentation, per client. CI-enforced. For web this also runs the integration project and covers the shared `core/` runtime (including `core/json` and `core/client`). | -| `npm run smoke` | End-to-end smokes through the built launcher (`--help` dispatch + prod cli/tui/web), plus two headless-Chromium smokes: a boot smoke that runs the prod web bundle and asserts a clean first render (no uncaught error — sync exception or unhandled rejection, how a Node built-in reaching the browser bundle manifests), and an **MCP Apps** smoke (`smoke:web:app`) that drives connect → open app → `data-app-status="ready"` against a composable App server, covering the sandbox proxy and UI-protocol bridge. | +| `npm run smoke` | End-to-end smokes through the built launcher (`--help` dispatch + prod cli/tui/web), plus two headless-Chromium smokes: a boot smoke that runs the prod web bundle and asserts a clean first render (no uncaught error — sync exception or unhandled rejection, how a Node built-in reaching the browser bundle manifests), and an **MCP Apps** smoke (`smoke:web:app`) that drives connect → open app → `data-app-status="ready"` against a composable App server, covering the sandbox proxy and UI-protocol bridge. A third (`smoke:web:elicit`) drives an **app-rendered elicitation** end to end — call the tool, answer inside the sandboxed app, see the app's `ElicitResult` reach the server — and then the same tool against a server that never advertised the capability, which must fall back to the native elicitation form. | | `npm run verify:build-gate` | Runs a real `vite build` with a Node built-in forced into the browser graph and asserts the build **fails** via the #1769 gate (which turns Vite's browser-externalization warning into a hard error). Guards against the warning phrasing drifting in a Vite bump and silently disabling the gate. Part of `npm run ci`. | | `npm run verify:format-coverage` | Parses the `format:check` globs out of every `package.json` (only those reachable from `validate`), enumerates all tracked source files, and **fails** listing any not covered by a glob — the durable guard for the "every first-party source file is format-gated" invariant (#1792). Runs first in `validate`. | | `npm run test:scripts` | Table-driven unit tests (`node --test`) for the guard's own pure parsers (`scripts/lib/npm-scripts.mjs`, `scripts/lib/tsc-program.mjs` + the exported helpers of `verify-typecheck-coverage.mjs` and `verify-dep-lockstep.mjs`), one case per rule they encode, plus `scripts/lib/resolve-node-bin.test.mjs` — the cross-platform bin resolver (#1939), pinned against the real `bin`/`exports` shapes of the packages the scripts actually spawn. Runs in `validate` — and `verify:typecheck-coverage` guards *this* gate in turn (reachable from `validate`, non-empty test set, every test file matched by the `test:scripts` glob), since `node --test` silently skips a file its glob misses and still exits 0. | diff --git a/clients/web/README.md b/clients/web/README.md index 0187385447..19f773856b 100644 --- a/clients/web/README.md +++ b/clients/web/README.md @@ -85,6 +85,15 @@ The Apps screen exposes a small, stable set of `data-testid` / `data-*` attribut | `data-testid="apps-messages"` | messages panel | `ui/message` submissions from the running view. | | `data-testid="apps-logs"` | app-logs panel | `notifications/message` log entries (default-expanded). | +App-rendered **elicitations** (#1854) render through the same `AppRenderer` but outside the Apps screen — one modal per request, from `AppElicitationHost` — and carry their own pair: + +| Attribute | Where | Meaning | +| --- | --- | --- | +| `data-testid="app-elicitation"` | the elicitation modal | One per in-flight app-rendered elicitation. **Absent** means the request was answered by the native elicitation form instead, which is what a driver asserts to prove the negotiation gate held. | +| `data-app-elicitation-status` | on `app-elicitation` | The same `AppRendererStatus` for that modal's app. `ready` is when the host forwards the `elicitation/create` through its bridge. | + +`scripts/smoke-web-elicitation.mjs` drives both halves against the public fixture (`test-servers/configs/app-elicitation-http.json` and its `-native-` sibling). + The renderer lifecycle itself is `AppRendererStatus` (`loading` | `ready` | `error`) reported via `AppRenderer`'s `onAppStatusChange`; the screen maps it to `data-app-status`. Resource-read failures (malformed/404 UI resource) are surfaced as a toast via the bridge factory's `onResourceError`; because the app never reaches `ready` in that case, a driver times out on `data-app-status` and reads the toast. ## Deep-link auto-connect diff --git a/clients/web/src/App.tsx b/clients/web/src/App.tsx index db29e06b99..c3531ceb93 100644 --- a/clients/web/src/App.tsx +++ b/clients/web/src/App.tsx @@ -1,4 +1,11 @@ -import { useCallback, useEffect, useMemo, useRef, useState } from "react"; +import { + useCallback, + useEffect, + useMemo, + useRef, + useState, + useSyncExternalStore, +} from "react"; import { Anchor, Box, @@ -134,6 +141,8 @@ import { import { clearScrollMemory } from "./hooks/useScrollMemory"; import type { AppRendererHandle } from "./components/elements/AppRenderer/AppRenderer"; import { createAppBridgeFactory } from "./components/elements/AppRenderer/createAppBridgeFactory"; +import { AppElicitationHost } from "./components/elements/AppElicitation/AppElicitationHost"; +import { AppElicitationController } from "./lib/appElicitationController"; import type { LogEntryData } from "./components/elements/LogEntry/LogEntry"; import { ServerConfigModal, @@ -792,6 +801,56 @@ function App() { [inspectorClient], ); + // App-rendered form elicitations (#1854). The controller is created once and + // handed to every InspectorClient at construction — its `render` is what opts + // this client into advertising the nested MCP Apps `elicitation` capability, + // which is why only the web client (the one with a sandbox) claims it. + const appElicitationControllerRef = useRef(null); + appElicitationControllerRef.current ??= new AppElicitationController(); + const appElicitationController = appElicitationControllerRef.current; + const appElicitations = useSyncExternalStore( + appElicitationController.subscribe, + appElicitationController.getEntries, + ); + // A SECOND factory, differing from `sandboxBridgeFactory` only in that it + // advertises `hostCapabilities.elicitation`. An App-tool frame is never handed + // an elicitation, so telling those apps otherwise would be a false claim. + const elicitationBridgeFactory = useMemo( + () => + createAppBridgeFactory({ + advertiseElicitation: true, + getClient: () => inspectorClient?.getAppRendererClient() ?? null, + readResource: async (uri) => { + if (!inspectorClient) throw new Error("No MCP client connected."); + const invocation = await inspectorClient.readResource(uri); + return invocation.result; + }, + // Unlike the Apps tab there is no persistent surface to show the + // failure on — the modal is about to be replaced by the native form — + // so the toast is the only place the user learns why. + onResourceError: (err) => { + notifications.show({ + title: "Elicitation app failed to load", + message: err.message, + color: "red", + }); + }, + }), + [inspectorClient], + ); + const handleAppElicitationSettle = useCallback( + (requestId: string, result: ElicitResult) => { + appElicitationController.settle(requestId, result); + }, + [appElicitationController], + ); + const handleAppElicitationFail = useCallback( + (requestId: string, error: Error) => { + appElicitationController.fail(requestId, error); + }, + [appElicitationController], + ); + const [managedToolsState, setManagedToolsState] = useState(null); const [managedPromptsState, setManagedPromptsState] = @@ -2410,6 +2469,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 }, + // Web only: hands form elicitations that name a `ui://` resource to the + // MCP App the server chose, and advertises the nested MCP Apps + // `elicitation` capability that makes a server willing to send one + // (#1854). The native elicitation queue stays the fallback. + appElicitation: appElicitationController.render, // 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 @@ -2511,6 +2575,7 @@ function App() { sessionStorageAdapter, onBeforeOAuthRedirect, clientConfig, + appElicitationController.render, ], ); @@ -4571,6 +4636,13 @@ function App() { onRefreshApps={onRefreshTools} /> + ({ elicitation: {} }), + request: async () => ({ + action: "accept", + content: { choice: "option-a" }, + }), + sendHostContextChange: async () => {}, + addEventListener: () => {}, + removeEventListener: () => {}, + teardownResource: async () => ({}), + close: async () => {}, + } as unknown as AppBridge; +} + +const okFactory: BridgeFactory = () => createMockBridge(); + +const failingFactory: BridgeFactory = () => + Promise.reject(new Error("Bridge connect failed: handshake timed out")); + +const meta: Meta = { + title: "Elements/AppElicitationHost", + component: AppElicitationHost, + args: { + sandboxPath: PLACEHOLDER_SANDBOX, + bridgeFactory: okFactory, + onSettle: fn(), + onFail: fn(), + }, + parameters: { layout: "fullscreen" }, +}; + +export default meta; +type Story = StoryObj; + +/** One server-attached app, waiting on the user inside the sandbox frame. */ +export const SingleRequest: Story = { + args: { entries: [entry("req-1")] }, + play: async ({ canvasElement }) => { + // Modals portal to document.body, so scope to the whole document. + const body = within(canvasElement.ownerDocument.body); + await expect(await body.findByText("Choose option A or B.")).toBeVisible(); + }, +}; + +/** + * Two elicitations in flight at once, each with its own frame and bridge — the + * request-scoped ownership the contract requires. + */ +export const ConcurrentRequests: Story = { + args: { + entries: [ + entry("req-1", "ui://demo/first.html"), + entry("req-2", "ui://demo/second.html"), + ], + }, +}; + +/** The app could not be brought up; the host falls back to the native form. */ +export const RenderFailure: Story = { + args: { entries: [entry("req-1")], bridgeFactory: failingFactory }, + play: async ({ canvasElement }) => { + const body = within(canvasElement.ownerDocument.body); + await expect(await body.findByText(/App failed to render/)).toBeVisible(); + }, +}; diff --git a/clients/web/src/components/elements/AppElicitation/AppElicitationHost.test.tsx b/clients/web/src/components/elements/AppElicitation/AppElicitationHost.test.tsx new file mode 100644 index 0000000000..3f17a19cb8 --- /dev/null +++ b/clients/web/src/components/elements/AppElicitation/AppElicitationHost.test.tsx @@ -0,0 +1,411 @@ +import { act } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import type { AppBridge } from "@modelcontextprotocol/ext-apps/app-bridge"; +import type { ElicitRequest, ElicitResult } from "@modelcontextprotocol/client"; +import { renderWithMantine, screen } from "../../../test/renderWithMantine"; +import type { AppElicitationEntry } from "../../../lib/appElicitationController"; +import type { BridgeFactory } from "../AppRenderer/AppRenderer"; +import { + APP_ELICITATION_INIT_TIMEOUT_MS, + AppElicitationHost, +} from "./AppElicitationHost"; + +const params: ElicitRequest["params"] = { + message: "Choose an option", + requestedSchema: { + type: "object", + properties: { choice: { type: "string" } }, + required: ["choice"], + }, +}; + +/** + * The renderer's bridge, reduced to what this component drives: the app's + * advertised capabilities and the elicitation round-trip. `emit` lets a test + * play the view's `initialized` signal, which is what triggers the send. + */ +function createMockBridge(options: { + elicitation?: boolean; + answer?: () => Promise; +}) { + const listeners: Record void)[]> = {}; + const answer = + options.answer ?? (() => Promise.resolve({ action: "cancel" })); + const request = vi.fn<(...args: unknown[]) => Promise>(() => + answer(), + ); + return { + bridge: { + getAppCapabilities: () => + options.elicitation === false ? {} : { elicitation: {} }, + request, + teardownResource: vi.fn().mockResolvedValue({}), + close: vi.fn().mockResolvedValue(undefined), + addEventListener: vi.fn( + (event: string, handler: (p: unknown) => void) => { + (listeners[event] ??= []).push(handler); + }, + ), + removeEventListener: vi.fn(), + } as unknown as AppBridge, + request, + emit: (event: string, payload?: unknown) => { + (listeners[event] ?? []).forEach((h) => h(payload)); + }, + }; +} + +function makeEntry( + requestId: string, + resourceUri = "ui://demo/choose-option.html", +): AppElicitationEntry { + return { + requestId, + resourceUri, + params, + signal: new AbortController().signal, + resolve: vi.fn(), + reject: vi.fn(), + }; +} + +/** Two microtasks settle the renderer's bridge promise chain (see AppRenderer). */ +async function flushAsync(): Promise { + await act(async () => { + await Promise.resolve(); + await Promise.resolve(); + }); +} + +describe("AppElicitationHost (#1854)", () => { + const onSettle = vi.fn(); + const onFail = vi.fn(); + + beforeEach(() => { + onSettle.mockReset(); + onFail.mockReset(); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + it("renders nothing and fails every entry when there is no sandbox", () => { + renderWithMantine( + , + ); + expect(screen.queryByTestId("app-elicitation")).toBeNull(); + expect(onFail).toHaveBeenCalledTimes(2); + expect(onFail.mock.calls[0][1].message).toMatch(/sandbox is not available/); + }); + + it("forwards the request through the app's bridge once it is ready and settles with the result", async () => { + const mock = createMockBridge({ + answer: () => + Promise.resolve({ action: "accept", content: { choice: "option-a" } }), + }); + const factory = vi.fn(() => mock.bridge) as unknown as BridgeFactory; + + renderWithMantine( + , + ); + await flushAsync(); + // Nothing is sent before the view says it is ready — an app that has not + // completed ui/initialize has no handler registered yet. + expect(mock.request).not.toHaveBeenCalled(); + + await act(async () => { + mock.emit("initialized"); + await Promise.resolve(); + }); + + expect(mock.request.mock.calls[0][0]).toEqual({ + method: "elicitation/create", + params, + }); + expect(onSettle).toHaveBeenCalledWith("req-1", { + action: "accept", + content: { choice: "option-a" }, + }); + expect(onFail).not.toHaveBeenCalled(); + }); + + it("loads the app from the elicitation's own resource URI", async () => { + const mock = createMockBridge({}); + const factory = vi.fn(() => mock.bridge) as unknown as BridgeFactory; + renderWithMantine( + , + ); + await flushAsync(); + expect(factory).toHaveBeenCalledWith(expect.anything(), { + kind: "resource", + resourceUri: "ui://demo/other.html", + title: params.message, + }); + }); + + it("gives each concurrent request its own frame and bridge", async () => { + const first = createMockBridge({}); + const second = createMockBridge({}); + const bridges = [first.bridge, second.bridge]; + const factory = vi.fn(() => bridges.shift()) as unknown as BridgeFactory; + + renderWithMantine( + , + ); + await flushAsync(); + expect(factory).toHaveBeenCalledTimes(2); + + // Only the second app answers; its result must be attributed to req-2. + await act(async () => { + second.emit("initialized"); + await Promise.resolve(); + }); + expect(first.request).not.toHaveBeenCalled(); + expect(onSettle).toHaveBeenCalledTimes(1); + expect(onSettle.mock.calls[0][0]).toBe("req-2"); + }); + + it("falls back when the app does not advertise elicitation", async () => { + const mock = createMockBridge({ elicitation: false }); + renderWithMantine( + mock.bridge) as unknown as BridgeFactory} + onSettle={onSettle} + onFail={onFail} + />, + ); + await flushAsync(); + await act(async () => { + mock.emit("initialized"); + await Promise.resolve(); + }); + expect(onSettle).not.toHaveBeenCalled(); + expect(onFail.mock.calls[0][1].message).toMatch( + /does not support elicitation/, + ); + }); + + it("falls back when the bridge request fails", async () => { + const mock = createMockBridge({ + answer: () => Promise.reject(new Error("bridge exploded")), + }); + renderWithMantine( + mock.bridge) as unknown as BridgeFactory} + onSettle={onSettle} + onFail={onFail} + />, + ); + await flushAsync(); + await act(async () => { + mock.emit("initialized"); + await Promise.resolve(); + await Promise.resolve(); + }); + expect(onFail).toHaveBeenCalledWith("req-1", expect.any(Error)); + expect(onFail.mock.calls[0][1].message).toMatch(/bridge exploded/); + }); + + it("wraps a non-Error rejection so the fallback still gets an Error", async () => { + // The rejection crosses a sandbox boundary; an app or bridge can reject + // with anything, and `onFail` must still hand the caller an Error. + const mock = createMockBridge({ + answer: () => Promise.reject("just a string"), + }); + renderWithMantine( + mock.bridge) as unknown as BridgeFactory} + onSettle={onSettle} + onFail={onFail} + />, + ); + await flushAsync(); + await act(async () => { + mock.emit("initialized"); + await Promise.resolve(); + await Promise.resolve(); + }); + expect(onFail.mock.calls[0][1]).toBeInstanceOf(Error); + expect(onFail.mock.calls[0][1].message).toBe("just a string"); + }); + + it("does not fall back on the init deadline once the request is in flight", async () => { + // The deadline bounds the HANDSHAKE only. A user taking longer than 15s to + // answer must not have their app yanked away. + vi.useFakeTimers(); + let answer: ((result: ElicitResult) => void) | undefined; + const mock = createMockBridge({ + answer: () => new Promise((resolve) => (answer = resolve)), + }); + renderWithMantine( + mock.bridge) as unknown as BridgeFactory} + onSettle={onSettle} + onFail={onFail} + />, + ); + await act(async () => { + await Promise.resolve(); + await Promise.resolve(); + mock.emit("initialized"); + await Promise.resolve(); + }); + expect(mock.request).toHaveBeenCalledTimes(1); + + await act(async () => { + await vi.advanceTimersByTimeAsync(APP_ELICITATION_INIT_TIMEOUT_MS * 2); + }); + expect(onFail).not.toHaveBeenCalled(); + + await act(async () => { + answer?.({ action: "accept", content: { choice: "option-a" } }); + await Promise.resolve(); + }); + expect(onSettle).toHaveBeenCalledWith("req-1", { + action: "accept", + content: { choice: "option-a" }, + }); + }); + + it("falls back when the renderer cannot build a bridge at all", async () => { + const factory = vi.fn(() => { + throw new Error("no connected MCP client"); + }) as unknown as BridgeFactory; + renderWithMantine( + , + ); + await flushAsync(); + expect(onFail).toHaveBeenCalled(); + expect(await screen.findByText(/App failed to render/)).toBeTruthy(); + }); + + it("falls back when the app never completes its handshake", async () => { + vi.useFakeTimers(); + const mock = createMockBridge({}); + renderWithMantine( + mock.bridge) as unknown as BridgeFactory} + onSettle={onSettle} + onFail={onFail} + />, + ); + await act(async () => { + await vi.advanceTimersByTimeAsync(APP_ELICITATION_INIT_TIMEOUT_MS + 1); + }); + expect(mock.request).not.toHaveBeenCalled(); + expect(onFail.mock.calls[0][1].message).toMatch(/did not initialize/); + }); + + it("falls back when the user dismisses the modal", async () => { + const user = userEvent.setup(); + const mock = createMockBridge({}); + renderWithMantine( + mock.bridge) as unknown as BridgeFactory} + onSettle={onSettle} + onFail={onFail} + />, + ); + await flushAsync(); + await user.click( + screen.getByRole("button", { name: /close and use the built-in/i }), + ); + // Dismissing is not an answer: the server still needs one, so this must be + // a fallback rather than a fabricated `cancel`. + expect(onSettle).not.toHaveBeenCalled(); + expect(onFail.mock.calls[0][1].message).toMatch(/dismissed/); + }); + + it("sends only one request even if the view signals ready twice", async () => { + const mock = createMockBridge({}); + renderWithMantine( + mock.bridge) as unknown as BridgeFactory} + onSettle={onSettle} + onFail={onFail} + />, + ); + await flushAsync(); + await act(async () => { + mock.emit("initialized"); + mock.emit("initialized"); + await Promise.resolve(); + }); + expect(mock.request).toHaveBeenCalledTimes(1); + }); + + it("passes decline and cancel through as completed answers", async () => { + const results: ElicitResult[] = [ + { action: "decline" }, + { action: "cancel" }, + ]; + for (const result of results) { + onSettle.mockReset(); + const mock = createMockBridge({ answer: () => Promise.resolve(result) }); + const { unmount } = renderWithMantine( + mock.bridge) as unknown as BridgeFactory} + onSettle={onSettle} + onFail={onFail} + />, + ); + await flushAsync(); + await act(async () => { + mock.emit("initialized"); + await Promise.resolve(); + }); + expect(onSettle).toHaveBeenCalledWith("req-1", result); + unmount(); + } + }); +}); diff --git a/clients/web/src/components/elements/AppElicitation/AppElicitationHost.tsx b/clients/web/src/components/elements/AppElicitation/AppElicitationHost.tsx new file mode 100644 index 0000000000..c66f932bec --- /dev/null +++ b/clients/web/src/components/elements/AppElicitation/AppElicitationHost.tsx @@ -0,0 +1,270 @@ +import { Alert, CloseButton, Group, Modal, Stack, Text } from "@mantine/core"; +import { useCallback, useEffect, useMemo, useRef, useState } from "react"; +import type { ElicitResult } from "@modelcontextprotocol/client"; +import type { AppElicitationEntry } from "../../../lib/appElicitationController"; +import { + AppRenderer, + type AppRenderSource, + type AppRendererHandle, + type AppRendererStatus, + type BridgeFactory, +} from "../AppRenderer/AppRenderer"; + +/** + * How long an app has to complete `ui/initialize` before the host gives up and + * lets the native elicitation UI take the request. + * + * Short on purpose, and unrelated to {@link APP_ELICITATION_TIMEOUT_MS} (which + * bounds the *answer*): nothing here waits on a human, so a sandbox that has + * not handshaken in this window is broken rather than slow, and the user is + * better served by the native form than by a spinner. + */ +export const APP_ELICITATION_INIT_TIMEOUT_MS = 15_000; + +/** + * Modal shell for one app-rendered elicitation. + * + * Deliberately headerless (`withCloseButton: false`, no `title`): concurrent + * elicitations mean two of these are open at once, and Mantine's modal header + * is a `
` — a second banner landmark, which axe flags as both + * `landmark-no-duplicate-banner` and `landmark-unique`. The title and the close + * affordance live in the body instead, and each dialog is named by its own + * `aria-label` so the two remain distinguishable. + */ +const ElicitationModal = Modal.Root.withProps({ + centered: true, + size: "lg", + closeOnClickOutside: false, +}); + +const ElicitationOverlay = Modal.Overlay.withProps({ + backgroundOpacity: 0.55, + blur: 2, +}); + +/** Title row: the server's prompt plus the dismiss control. */ +const TitleRow = Group.withProps({ + justify: "space-between", + align: "flex-start", + wrap: "nowrap", + gap: "sm", +}); + +const TitleText = Text.withProps({ + fw: 600, + size: "md", +}); + +/** Fixed-height frame the app renders into; apps size themselves within it. */ +const FrameBox = Stack.withProps({ + h: 360, + gap: 0, +}); + +const PromptText = Text.withProps({ + size: "sm", + c: "dimmed", +}); + +function toError(err: unknown): Error { + return err instanceof Error ? err : new Error(String(err)); +} + +export interface AppElicitationHostProps { + /** Elicitations currently awaiting an app answer, oldest first. */ + entries: AppElicitationEntry[]; + /** + * The inspector's sandbox-proxy URL. Absent means the sandbox controller is + * not running, so nothing can be rendered — every entry falls back at once. + */ + sandboxPath?: string; + /** Builds the per-app bridge. Must advertise `hostCapabilities.elicitation`. */ + bridgeFactory: BridgeFactory; + /** The app answered: hand the standard result back to the server. */ + onSettle: (requestId: string, result: ElicitResult) => void; + /** The app could not answer: fall back to the native elicitation UI. */ + onFail: (requestId: string, error: Error) => void; +} + +/** + * Renders every pending app-rendered elicitation (#1854), one modal and one + * bridge per request. + * + * Keyed by `requestId` rather than by resource URI so that two concurrent + * requests — even for the SAME app — get distinct React subtrees, distinct + * iframes and distinct bridges. That is what makes the ownership request-scoped + * in practice, not just on paper. + */ +export function AppElicitationHost({ + entries, + sandboxPath, + bridgeFactory, + onSettle, + onFail, +}: AppElicitationHostProps) { + // No sandbox → nothing can render. Fail every entry immediately rather than + // showing an empty modal the user cannot act on. + useEffect(() => { + if (sandboxPath) return; + for (const entry of entries) { + onFail( + entry.requestId, + new Error("MCP App sandbox is not available in this session"), + ); + } + }, [entries, sandboxPath, onFail]); + + if (!sandboxPath) return null; + + return ( + <> + {entries.map((entry) => ( + + ))} + + ); +} + +interface AppElicitationFrameProps { + entry: AppElicitationEntry; + sandboxPath: string; + bridgeFactory: BridgeFactory; + onSettle: (requestId: string, result: ElicitResult) => void; + onFail: (requestId: string, error: Error) => void; +} + +/** + * One request: mount the app, and the moment its view reports `ready`, forward + * the original `elicitation/create` through THAT app's bridge. + * + * Every failure route ends in `onFail`, which is the fallback signal — an + * unreachable sandbox, a view that never handshakes, an app with no elicitation + * capability, a bridge error, or the user dismissing the modal. + */ +function AppElicitationFrame({ + entry, + sandboxPath, + bridgeFactory, + onSettle, + onFail, +}: AppElicitationFrameProps) { + const rendererRef = useRef(null); + // Guards the one-shot send: `ready` can fire again after a bridge rebuild, + // and a second `elicitation/create` for the same server request would be a + // duplicate the server never asked for. + const sentRef = useRef(false); + const [status, setStatus] = useState("loading"); + + const source = useMemo( + () => ({ + kind: "resource", + resourceUri: entry.resourceUri, + title: entry.params.message, + }), + [entry.resourceUri, entry.params.message], + ); + + const fail = useCallback( + (error: Error) => { + onFail(entry.requestId, error); + }, + [entry.requestId, onFail], + ); + + // Dismissing is not an answer: the server is still waiting, so hand the + // request to the native elicitation UI rather than inventing a `cancel`. + const dismiss = useCallback( + () => fail(new Error("App-rendered elicitation dismissed")), + [fail], + ); + + // Initialization deadline. Cleared as soon as the request goes out, so it + // only ever bounds the handshake and never the user's answer. + useEffect(() => { + const timer = window.setTimeout(() => { + if (sentRef.current) return; + fail(new Error("MCP App did not initialize in time")); + }, APP_ELICITATION_INIT_TIMEOUT_MS); + return () => window.clearTimeout(timer); + }, [fail]); + + const handleStatus = useCallback( + (next: AppRendererStatus) => { + setStatus(next); + if (next === "error") { + fail(new Error("MCP App failed to render")); + return; + } + if (next !== "ready" || sentRef.current) return; + sentRef.current = true; + const handle = rendererRef.current; + /* v8 ignore next 4 -- defensive: `ready` is dispatched from the renderer's + own bridge callback, so its imperative handle is always attached by the + time this runs. */ + if (!handle) { + fail(new Error("MCP App renderer is unavailable")); + return; + } + handle + .requestElicitation(entry.params) + .then((result) => onSettle(entry.requestId, result)) + .catch((err: unknown) => fail(toError(err))); + }, + [entry.params, entry.requestId, fail, onSettle], + ); + + return ( + + + {/* `aria-label` and the `data-*` attributes go on the CONTENT (the + role="dialog" element), not the Root — the Root is only a portal + wrapper, so a name placed there never reaches the dialog. Named per + request rather than per prompt: two concurrent elicitations can carry + the same message, and two identically-named dialogs are what + `landmark-unique` rejects. */} + + + + + {entry.params.message} + + + + Answering through the server-provided MCP App. + + {status === "error" ? ( + + Falling back to the built-in elicitation form. + + ) : ( + + + + )} + + + + + ); +} diff --git a/clients/web/src/components/elements/AppRenderer/AppRenderer.stories.tsx b/clients/web/src/components/elements/AppRenderer/AppRenderer.stories.tsx index b1df1c0bcc..c24fa5a04c 100644 --- a/clients/web/src/components/elements/AppRenderer/AppRenderer.stories.tsx +++ b/clients/web/src/components/elements/AppRenderer/AppRenderer.stories.tsx @@ -61,7 +61,7 @@ const meta: Meta = { component: AppRenderer, args: { sandboxPath: PLACEHOLDER_SANDBOX, - tool: cohortTool, + source: { kind: "tool", tool: cohortTool }, onError: fn(), }, parameters: { diff --git a/clients/web/src/components/elements/AppRenderer/AppRenderer.test.tsx b/clients/web/src/components/elements/AppRenderer/AppRenderer.test.tsx index c309e43ae4..a9a167cc2e 100644 --- a/clients/web/src/components/elements/AppRenderer/AppRenderer.test.tsx +++ b/clients/web/src/components/elements/AppRenderer/AppRenderer.test.tsx @@ -85,7 +85,7 @@ describe("AppRenderer", () => { renderWithMantine( asBridge(bridge)} />, ); @@ -99,7 +99,10 @@ describe("AppRenderer", () => { renderWithMantine( asBridge(bridge)} />, ); @@ -112,14 +115,14 @@ describe("AppRenderer", () => { renderWithMantine( , ); await flushAsync(); expect(factory).toHaveBeenCalledTimes(1); expect(factory.mock.calls[0]?.[0]).toBeInstanceOf(HTMLIFrameElement); - expect(factory.mock.calls[0]?.[1]).toBe(tool); + expect(factory.mock.calls[0]?.[1]).toEqual({ kind: "tool", tool }); }); it("forwards sendToolInput through the bridge once initialized", async () => { @@ -129,7 +132,7 @@ describe("AppRenderer", () => { asBridge(bridge)} />, ); @@ -153,7 +156,7 @@ describe("AppRenderer", () => { asBridge(bridge)} />, ); @@ -173,7 +176,7 @@ describe("AppRenderer", () => { asBridge(bridge)} />, ); @@ -205,7 +208,7 @@ describe("AppRenderer", () => { asBridge(bridge)} />, ); @@ -228,7 +231,7 @@ describe("AppRenderer", () => { asBridge(bridge)} />, ); @@ -247,7 +250,7 @@ describe("AppRenderer", () => { renderWithMantine( asBridge(bridge)} onSizeChange={onSizeChange} />, @@ -264,7 +267,7 @@ describe("AppRenderer", () => { renderWithMantine( asBridge(bridge)} />, ); @@ -283,7 +286,7 @@ describe("AppRenderer", () => { renderWithMantine( asBridge(bridge)} displayMode="inline" onRequestDisplayMode={onRequestDisplayMode} @@ -301,7 +304,7 @@ describe("AppRenderer", () => { renderWithMantine( asBridge(bridge)} displayMode="fullscreen" />, @@ -317,7 +320,7 @@ describe("AppRenderer", () => { renderWithMantine( asBridge(bridge)} />, ); @@ -334,7 +337,7 @@ describe("AppRenderer", () => { asBridge(bridge)} partialInputs={[{ city: "N" }, { city: "New" }]} />, @@ -363,7 +366,7 @@ describe("AppRenderer", () => { asBridge(bridge)} />, ); @@ -381,7 +384,7 @@ describe("AppRenderer", () => { renderWithMantine( asBridge(bridge)} onLog={onLog} />, @@ -398,7 +401,7 @@ describe("AppRenderer", () => { renderWithMantine( asBridge(bridge)} />, ); @@ -415,7 +418,7 @@ describe("AppRenderer", () => { renderWithMantine( asBridge(bridge)} onMessage={onMessage} />, @@ -434,7 +437,7 @@ describe("AppRenderer", () => { renderWithMantine( asBridge(bridge)} />, ); @@ -452,7 +455,7 @@ describe("AppRenderer", () => { const { rerender } = renderWithMantine( , @@ -463,7 +466,7 @@ describe("AppRenderer", () => { rerender( , @@ -480,7 +483,7 @@ describe("AppRenderer", () => { const { rerender } = renderWithMantine( , @@ -491,7 +494,7 @@ describe("AppRenderer", () => { rerender( , @@ -523,7 +526,7 @@ describe("AppRenderer", () => { renderWithMantine( asBridge(bridge)} />, ); @@ -564,7 +567,7 @@ describe("AppRenderer", () => { renderWithMantine( asBridge(bridge)} />, ); @@ -588,7 +591,7 @@ describe("AppRenderer", () => { const { unmount } = renderWithMantine( asBridge(bridge)} />, ); @@ -665,7 +668,7 @@ describe("AppRenderer", () => { renderWithMantine( asBridge(bridge)} containerRef={{ current: container }} />, @@ -684,7 +687,7 @@ describe("AppRenderer", () => { renderWithMantine( asBridge(bridge)} containerRef={{ current: container }} />, @@ -703,7 +706,7 @@ describe("AppRenderer", () => { renderWithMantine( asBridge(bridge)} containerRef={{ current: container }} />, @@ -721,7 +724,7 @@ describe("AppRenderer", () => { renderWithMantine( asBridge(bridge)} containerRef={{ current: container }} />, @@ -752,7 +755,7 @@ describe("AppRenderer", () => { renderWithMantine( asBridge(bridge)} containerRef={{ current: container }} />, @@ -766,7 +769,7 @@ describe("AppRenderer", () => { renderWithMantine( asBridge(bridge)} />, ); @@ -779,7 +782,7 @@ describe("AppRenderer", () => { const { unmount } = renderWithMantine( asBridge(bridge)} />, ); @@ -807,7 +810,7 @@ describe("AppRenderer", () => { , @@ -837,7 +840,7 @@ describe("AppRenderer", () => { asBridge(bridge)} />, ); @@ -864,7 +867,7 @@ describe("AppRenderer", () => { asBridge(bridge)} />, ); @@ -901,7 +904,7 @@ describe("AppRenderer", () => { , ); @@ -925,7 +928,7 @@ describe("AppRenderer", () => { const { unmount } = renderWithMantine( asBridge(bridge)} />, ); @@ -949,7 +952,7 @@ describe("AppRenderer", () => { const { unmount } = renderWithMantine( , ); @@ -972,7 +975,7 @@ describe("AppRenderer", () => { renderWithMantine( asBridge(bridge)} onAppStatusChange={onAppStatusChange} />, @@ -993,7 +996,7 @@ describe("AppRenderer", () => { renderWithMantine( , @@ -1010,7 +1013,7 @@ describe("AppRenderer", () => { renderWithMantine( , @@ -1027,7 +1030,7 @@ describe("AppRenderer", () => { renderWithMantine( , @@ -1044,7 +1047,7 @@ describe("AppRenderer", () => { renderWithMantine( , @@ -1059,7 +1062,7 @@ describe("AppRenderer", () => { renderWithMantine( , ); @@ -1078,7 +1081,7 @@ describe("AppRenderer", () => { const { rerender } = renderWithMantine( , ); @@ -1087,7 +1090,7 @@ describe("AppRenderer", () => { rerender( , ); @@ -1117,7 +1120,7 @@ describe("AppRenderer", () => { const { rerender } = renderWithMantine( , ); @@ -1126,14 +1129,17 @@ describe("AppRenderer", () => { rerender( , ); await flushAsync(); expect(factory).toHaveBeenCalledTimes(2); - expect(factory.mock.calls[1]?.[1]).toBe(otherTool); + expect(factory.mock.calls[1]?.[1]).toEqual({ + kind: "tool", + tool: otherTool, + }); expect(first.teardownResource).toHaveBeenCalledTimes(1); expect(first.close).toHaveBeenCalledTimes(1); }); @@ -1144,7 +1150,7 @@ describe("AppRenderer", () => { const { unmount } = renderWithMantine( asBridge(bridge)} />, ); @@ -1164,7 +1170,7 @@ describe("AppRenderer", () => { const { unmount } = renderWithMantine( asBridge(bridge)} />, ); @@ -1177,4 +1183,114 @@ describe("AppRenderer", () => { expect(bridge.teardownResource).toHaveBeenCalledTimes(1); expect(bridge.close).toHaveBeenCalledTimes(1); }); + + describe("app-rendered elicitation (#1854)", () => { + /** A bridge that can answer an `elicitation/create` sent through it. */ + function elicitationBridge( + result: unknown = { action: "accept", content: { choice: "a" } }, + ) { + const bridge = createMockBridge(); + const withElicitation = Object.assign(bridge, { + getAppCapabilities: () => ({ elicitation: {} }), + request: vi.fn().mockResolvedValue(result), + }); + return withElicitation; + } + + it("renders an app named by resource URI, using the URI as the frame title", () => { + const bridge = createMockBridge(); + renderWithMantine( + asBridge(bridge)} + />, + ); + expect(screen.getByTitle("ui://demo/pick.html")).toBeTruthy(); + }); + + it("prefers an explicit title over the URI", () => { + const bridge = createMockBridge(); + renderWithMantine( + asBridge(bridge)} + />, + ); + expect(screen.getByTitle("Choose an option")).toBeTruthy(); + }); + + it("sends the request through the live bridge once the view is initialized", async () => { + const bridge = elicitationBridge(); + const ref = createRef(); + renderWithMantine( + asBridge(bridge)} + />, + ); + await flushAsync(); + act(() => bridge.emit("initialized")); + await expect( + ref.current!.requestElicitation({ + message: "Choose", + requestedSchema: { type: "object", properties: {} }, + }), + ).resolves.toEqual({ action: "accept", content: { choice: "a" } }); + }); + + it("rejects rather than buffering when the view is not ready", async () => { + // Unlike tool input/result there is a server waiting on this, so a + // caller that arrives early must learn now and fall back. + const bridge = elicitationBridge(); + const ref = createRef(); + renderWithMantine( + asBridge(bridge)} + />, + ); + await flushAsync(); + await expect( + ref.current!.requestElicitation({ + message: "Choose", + requestedSchema: { type: "object", properties: {} }, + }), + ).rejects.toThrow(/not ready/); + expect(bridge.request).not.toHaveBeenCalled(); + }); + + it("keeps a live bridge when the source object is recreated with the same tool", async () => { + // A caller writing the source inline produces a fresh object every + // render; rebuilding on that double-loads the sandbox. + const bridge = createMockBridge(); + const factory = vi.fn(() => asBridge(bridge)); + const { rerender } = renderWithMantine( + , + ); + await flushAsync(); + rerender( + , + ); + await flushAsync(); + expect(factory).toHaveBeenCalledTimes(1); + }); + }); }); diff --git a/clients/web/src/components/elements/AppRenderer/AppRenderer.tsx b/clients/web/src/components/elements/AppRenderer/AppRenderer.tsx index 19eb85838c..b14cd1d678 100644 --- a/clients/web/src/components/elements/AppRenderer/AppRenderer.tsx +++ b/clients/web/src/components/elements/AppRenderer/AppRenderer.tsx @@ -15,9 +15,18 @@ import type { } from "@modelcontextprotocol/ext-apps/app-bridge"; import type { CallToolResult, + ElicitRequest, + ElicitResult, LoggingMessageNotification, - Tool, } from "@modelcontextprotocol/client"; +import { requestAppElicitation } from "./requestAppElicitation"; +import { + appSourceTitle, + sameAppSource, + type AppRenderSource, +} from "./appRenderSource"; + +export type { AppRenderSource } from "./appRenderSource"; import { currentStyles, currentTheme, @@ -32,13 +41,23 @@ import { */ export type BridgeFactory = ( iframe: HTMLIFrameElement, - tool: Tool, + source: AppRenderSource, ) => AppBridge | Promise; export interface AppRendererHandle { sendToolInput(args: Record): Promise; sendToolResult(result: CallToolResult): Promise; sendToolCancelled(reason: string): Promise; + /** + * Forward a form-mode `elicitation/create` through THIS renderer's bridge and + * resolve with the app's standard `ElicitResult` (#1854). + * + * Rejects — rather than resolving with anything invented — when the app is not + * live, did not advertise `elicitation`, or fails the request, because the + * caller's contract is that a rejection means "fall back to the native UI" + * while a resolution is a real user decision. + */ + requestElicitation(params: ElicitRequest["params"]): Promise; teardown(): Promise; } @@ -54,7 +73,7 @@ export type AppRendererStatus = "loading" | "ready" | "error"; export interface AppRendererProps { sandboxPath: string; - tool: Tool; + source: AppRenderSource; bridgeFactory: BridgeFactory; onError?: (err: Error) => void; /** @@ -135,7 +154,7 @@ async function disposeBridge(bridge: AppBridge): Promise { /** * Bridge lifecycle (the interlocking refs below): * - * mount ─▶ build (buildId++) ─▶ factory(iframe,tool) ─async─▶ bridgeRef set + * mount ─▶ build (buildId++) ─▶ factory(iframe,source) ─async─▶ bridgeRef set * │ on "initialized" * ▼ → flushPending * cleanup ─▶ scheduleDispose() ──microtask──▶ dispose (unless cancelled) @@ -154,7 +173,7 @@ async function disposeBridge(bridge: AppBridge): Promise { */ export function AppRenderer({ sandboxPath, - tool, + source, bridgeFactory, onError, onAppStatusChange, @@ -182,7 +201,7 @@ export function AppRenderer({ const lastDepsRef = useRef<{ bridgeFactory: BridgeFactory; sandboxPath: string; - tool: Tool; + source: AppRenderSource; } | null>(null); const onErrorRef = useRef(onError); const onAppStatusChangeRef = useRef(onAppStatusChange); @@ -268,7 +287,7 @@ export function AppRenderer({ prev !== null && prev.bridgeFactory === bridgeFactory && prev.sandboxPath === sandboxPath && - prev.tool === tool; + sameAppSource(prev.source, source); // A disposal scheduled by the immediately-preceding cleanup means we are in // a synchronous re-setup. If the inputs are identical (StrictMode's @@ -298,7 +317,7 @@ export function AppRenderer({ if (old) void disposeBridge(old); } - lastDepsRef.current = { bridgeFactory, sandboxPath, tool }; + lastDepsRef.current = { bridgeFactory, sandboxPath, source }; const buildId = ++buildIdRef.current; teardownStartedRef.current = false; initializedRef.current = false; @@ -311,7 +330,7 @@ export function AppRenderer({ let pending: Promise; try { - pending = Promise.resolve(bridgeFactory(iframe, tool)); + pending = Promise.resolve(bridgeFactory(iframe, source)); } catch (err) { onAppStatusChangeRef.current?.("error"); onErrorRef.current?.(toError(err)); @@ -391,7 +410,7 @@ export function AppRenderer({ }, [ bridgeFactory, sandboxPath, - tool, + source, containerRef, flushPending, scheduleDispose, @@ -489,6 +508,18 @@ export function AppRenderer({ pendingResultRef.current = result; flushPending(); }, + async requestElicitation(params) { + const bridge = bridgeRef.current; + // Not "not ready yet, buffer it" like tool input/result: an elicitation + // has a server waiting on it, so a caller that arrives before the + // handshake must learn that now and fall back, not block. + if (!bridge || !initializedRef.current) { + throw new Error( + "MCP App is not ready to receive an elicitation request", + ); + } + return requestAppElicitation(bridge, params); + }, async sendToolCancelled(reason) { const bridge = bridgeRef.current; if (!bridge) return; @@ -528,7 +559,7 @@ export function AppRenderer({ component="iframe" ref={iframeRef} src={sandboxPath} - title={tool.title ?? tool.name} + title={appSourceTitle(source)} w="100%" h="100%" bd={0} diff --git a/clients/web/src/components/elements/AppRenderer/appCapabilities.test.ts b/clients/web/src/components/elements/AppRenderer/appCapabilities.test.ts new file mode 100644 index 0000000000..6e15829e6e --- /dev/null +++ b/clients/web/src/components/elements/AppRenderer/appCapabilities.test.ts @@ -0,0 +1,94 @@ +import { describe, it, expect, vi } from "vitest"; +import type { AppBridge } from "@modelcontextprotocol/ext-apps/app-bridge"; +import type { JSONRPCMessage, Transport } from "@modelcontextprotocol/client"; +import { + appAdvertisesElicitation, + observeAppCapabilities, +} from "./appCapabilities"; + +function makeBridge(parsed?: Record): AppBridge { + return { getAppCapabilities: () => parsed } as unknown as AppBridge; +} + +function makeTransport(): { + transport: Transport; + inner: ReturnType; +} { + const inner = vi.fn(); + const transport = { onmessage: inner } as unknown as Transport; + return { transport, inner }; +} + +function initializeFrame(appCapabilities: unknown): JSONRPCMessage { + return { + jsonrpc: "2.0", + id: 1, + method: "ui/initialize", + params: { appCapabilities }, + } as unknown as JSONRPCMessage; +} + +describe("appCapabilities (#1854)", () => { + it("records the raw ui/initialize capabilities the bridge's schema strips", () => { + // The whole reason this module exists: ext-apps 1.7.5 parses away the + // `elicitation` key, so an app that DID advertise it reads as one that did + // not — and every negotiated elicitation silently becomes a fallback. + const bridge = makeBridge({}); + const { transport, inner } = makeTransport(); + observeAppCapabilities(bridge, transport); + + expect(appAdvertisesElicitation(bridge)).toBe(false); + transport.onmessage?.(initializeFrame({ elicitation: {} })); + expect(appAdvertisesElicitation(bridge)).toBe(true); + // The bridge's own handler still runs — observing must not swallow. + expect(inner).toHaveBeenCalledTimes(1); + }); + + it("prefers the bridge's own value once ext-apps carries it", () => { + const bridge = makeBridge({ elicitation: {} }); + expect(appAdvertisesElicitation(bridge)).toBe(true); + }); + + it("is false for an app that advertised no elicitation", () => { + const bridge = makeBridge({ availableDisplayModes: ["inline"] }); + const { transport } = makeTransport(); + observeAppCapabilities(bridge, transport); + transport.onmessage?.(initializeFrame({ availableDisplayModes: [] })); + expect(appAdvertisesElicitation(bridge)).toBe(false); + }); + + it("ignores other methods and non-object capabilities", () => { + const bridge = makeBridge(undefined); + const { transport } = makeTransport(); + observeAppCapabilities(bridge, transport); + transport.onmessage?.({ + jsonrpc: "2.0", + method: "ui/notifications/initialized", + } as JSONRPCMessage); + transport.onmessage?.(initializeFrame("not-an-object")); + transport.onmessage?.(initializeFrame(null)); + expect(appAdvertisesElicitation(bridge)).toBe(false); + }); + + it("keeps bridges independent", () => { + const a = makeBridge({}); + const b = makeBridge({}); + const first = makeTransport(); + const second = makeTransport(); + observeAppCapabilities(a, first.transport); + observeAppCapabilities(b, second.transport); + first.transport.onmessage?.(initializeFrame({ elicitation: {} })); + expect(appAdvertisesElicitation(a)).toBe(true); + expect(appAdvertisesElicitation(b)).toBe(false); + }); + + it("tolerates a transport with no prior handler", () => { + const bridge = makeBridge({}); + const transport = {} as unknown as Transport; + observeAppCapabilities(bridge, transport); + expect(() => + transport.onmessage?.(initializeFrame({ elicitation: {} })), + ).not.toThrow(); + expect(appAdvertisesElicitation(bridge)).toBe(true); + }); +}); diff --git a/clients/web/src/components/elements/AppRenderer/appCapabilities.ts b/clients/web/src/components/elements/AppRenderer/appCapabilities.ts new file mode 100644 index 0000000000..4ff832c12f --- /dev/null +++ b/clients/web/src/components/elements/AppRenderer/appCapabilities.ts @@ -0,0 +1,67 @@ +import type { AppBridge } from "@modelcontextprotocol/ext-apps/app-bridge"; +import type { Transport } from "@modelcontextprotocol/client"; + +/** + * The app capabilities exactly as the view sent them, per bridge. + * + * `AppBridge.getAppCapabilities()` cannot be used for `elicitation` (#1854): + * ext-apps 1.7.5 parses the view's `ui/initialize` params through + * `McpUiAppCapabilitiesSchema`, a plain Zod object, so any key that schema does + * not declare is **stripped before the bridge stores it**. `elicitation` is + * exactly such a key — it is what ext-apps#733 adds — so an app that correctly + * advertises it looks, through the bridge's own accessor, like an app that did + * not. That silently turns every negotiated elicitation into a native-UI + * fallback, which is indistinguishable from "the feature is off". + * + * So the raw frame is observed on the way in and recorded here. A WeakMap keeps + * this per-bridge (never global) and lets the entry die with the bridge. + * + * Delete this module when ext-apps ships #733: `getAppCapabilities()` will + * carry `elicitation` itself, and {@link appAdvertisesElicitation} already + * prefers that value. + */ +const rawAppCapabilities = new WeakMap>(); + +/** Shape of the `ui/initialize` frame this reads — nothing else is touched. */ +interface UiInitializeFrame { + method?: unknown; + params?: { appCapabilities?: unknown }; +} + +/** + * Wrap a connected bridge's transport so the view's `ui/initialize` params are + * recorded before the bridge parses them. + * + * Call AFTER `bridge.connect(transport)` — `connect` installs the handler this + * wraps. That ordering is safe: the view cannot send `ui/initialize` until the + * host has pushed its HTML into the sandbox, which happens later still. + */ +export function observeAppCapabilities( + bridge: AppBridge, + transport: Transport, +): void { + const inner = transport.onmessage?.bind(transport); + transport.onmessage = (message, extra) => { + const frame = message as UiInitializeFrame; + if (frame.method === "ui/initialize") { + const advertised = frame.params?.appCapabilities; + if (typeof advertised === "object" && advertised !== null) { + rawAppCapabilities.set(bridge, advertised as Record); + } + } + inner?.(message, extra); + }; +} + +/** + * Whether the app running on this bridge advertised the `elicitation` + * capability. Prefers the bridge's own accessor (correct once ext-apps#733 + * ships) and falls back to the observed raw frame. + */ +export function appAdvertisesElicitation(bridge: AppBridge): boolean { + const parsed = bridge.getAppCapabilities() as + | Record + | undefined; + if (parsed?.elicitation) return true; + return Boolean(rawAppCapabilities.get(bridge)?.elicitation); +} diff --git a/clients/web/src/components/elements/AppRenderer/appRenderSource.test.ts b/clients/web/src/components/elements/AppRenderer/appRenderSource.test.ts new file mode 100644 index 0000000000..cfb61f0997 --- /dev/null +++ b/clients/web/src/components/elements/AppRenderer/appRenderSource.test.ts @@ -0,0 +1,86 @@ +import { describe, it, expect } from "vitest"; +import type { Tool } from "@modelcontextprotocol/client"; +import { + appSourceTitle, + sameAppSource, + type AppRenderSource, +} from "./appRenderSource"; + +const tool: Tool = { + name: "cohort_app", + title: "Cohort App", + inputSchema: { type: "object" }, +}; +const other: Tool = { name: "other_app", inputSchema: { type: "object" } }; + +const toolSource: AppRenderSource = { kind: "tool", tool }; +const resourceSource: AppRenderSource = { + kind: "resource", + resourceUri: "ui://demo/pick.html", +}; + +describe("appRenderSource (#1854)", () => { + describe("sameAppSource", () => { + it("is true for the identical object", () => { + expect(sameAppSource(toolSource, toolSource)).toBe(true); + }); + + it("is true for a re-created wrapper around the same tool", () => { + // The reason the comparison is not `===`: a caller writing the source + // inline makes a new object each render, and rebuilding on that + // double-loads the sandbox. + expect(sameAppSource(toolSource, { kind: "tool", tool })).toBe(true); + }); + + it("is false when the Tool identity changes", () => { + // Preserved from before the union existed: a re-listed tool rebuilds. + expect(sameAppSource(toolSource, { kind: "tool", tool: other })).toBe( + false, + ); + expect( + sameAppSource(toolSource, { kind: "tool", tool: { ...tool } }), + ).toBe(false); + }); + + it("compares resource sources by URI and title", () => { + expect( + sameAppSource(resourceSource, { + kind: "resource", + resourceUri: "ui://demo/pick.html", + }), + ).toBe(true); + expect( + sameAppSource(resourceSource, { + kind: "resource", + resourceUri: "ui://demo/other.html", + }), + ).toBe(false); + expect( + sameAppSource(resourceSource, { + kind: "resource", + resourceUri: "ui://demo/pick.html", + title: "Pick one", + }), + ).toBe(false); + }); + + it("is false across kinds", () => { + expect(sameAppSource(toolSource, resourceSource)).toBe(false); + expect(sameAppSource(resourceSource, toolSource)).toBe(false); + }); + }); + + describe("appSourceTitle", () => { + it("prefers a tool's title, falling back to its name", () => { + expect(appSourceTitle(toolSource)).toBe("Cohort App"); + expect(appSourceTitle({ kind: "tool", tool: other })).toBe("other_app"); + }); + + it("prefers an explicit resource title, falling back to the URI", () => { + expect(appSourceTitle(resourceSource)).toBe("ui://demo/pick.html"); + expect( + appSourceTitle({ ...resourceSource, title: "Choose an option" }), + ).toBe("Choose an option"); + }); + }); +}); diff --git a/clients/web/src/components/elements/AppRenderer/appRenderSource.ts b/clients/web/src/components/elements/AppRenderer/appRenderSource.ts new file mode 100644 index 0000000000..3314311507 --- /dev/null +++ b/clients/web/src/components/elements/AppRenderer/appRenderSource.ts @@ -0,0 +1,49 @@ +import type { Tool } from "@modelcontextprotocol/client"; + +/** + * What the renderer loads into the sandbox. + * + * An App tool names its UI resource through `_meta.ui.resourceUri`, but an + * app-rendered elicitation (#1854) has no tool at all — the server names the + * resource on the `elicitation/create` request itself. The source is therefore + * a union rather than a `Tool`, so the same renderer, bridge factory and + * sandbox lifecycle serve both without either faking the other's shape. + * + * Lives beside `AppRenderer` rather than in it because a module that exports a + * component may export nothing else (the react-refresh rule). + */ +export type AppRenderSource = + | { readonly kind: "tool"; readonly tool: Tool } + | { + readonly kind: "resource"; + /** Absolute `ui://` URI of the app to load. */ + readonly resourceUri: string; + /** Frame title; falls back to the URI. */ + readonly title?: string; + }; + +/** + * Whether two sources name the same app, so the renderer can keep a live bridge + * instead of rebuilding it. + * + * Identity alone is not enough: a caller that writes the source inline produces + * a fresh object every render, and rebuilding on that double-loads the sandbox + * and races the app's handshake (the failure AppRenderer's reuse dance exists to + * avoid). For a tool the comparison stays *identity of the Tool*, exactly as + * before this union existed, so a re-listed tool still rebuilds. + */ +export function sameAppSource(a: AppRenderSource, b: AppRenderSource): boolean { + if (a === b) return true; + if (a.kind === "tool" && b.kind === "tool") return a.tool === b.tool; + if (a.kind === "resource" && b.kind === "resource") { + return a.resourceUri === b.resourceUri && a.title === b.title; + } + return false; +} + +/** The iframe's accessible name for a source. */ +export function appSourceTitle(source: AppRenderSource): string { + return source.kind === "tool" + ? (source.tool.title ?? source.tool.name) + : (source.title ?? source.resourceUri); +} diff --git a/clients/web/src/components/elements/AppRenderer/createAppBridgeFactory.test.ts b/clients/web/src/components/elements/AppRenderer/createAppBridgeFactory.test.ts index 13fa9f5523..cccdd33dc7 100644 --- a/clients/web/src/components/elements/AppRenderer/createAppBridgeFactory.test.ts +++ b/clients/web/src/components/elements/AppRenderer/createAppBridgeFactory.test.ts @@ -113,7 +113,7 @@ describe("createAppBridgeFactory", () => { getClient: () => null, readResource: vi.fn(), }); - await expect(factory(makeIframe(), tool)).rejects.toThrow( + await expect(factory(makeIframe(), { kind: "tool", tool })).rejects.toThrow( /no connected MCP client/, ); }); @@ -123,7 +123,9 @@ describe("createAppBridgeFactory", () => { getClient: () => fakeClient, readResource: vi.fn(), }); - await expect(factory(makeIframe(false), tool)).rejects.toThrow(/no window/); + await expect( + factory(makeIframe(false), { kind: "tool", tool }), + ).rejects.toThrow(/no window/); }); it("constructs the bridge with the client, host info, capabilities and theme, then connects", async () => { @@ -134,7 +136,7 @@ describe("createAppBridgeFactory", () => { getClient: () => fakeClient, readResource: vi.fn().mockResolvedValue(uiResource("

hi

")), }); - await factory(makeIframe(), tool); + await factory(makeIframe(), { kind: "tool", tool }); expect(bridgeInstances).toHaveLength(1); const bridge = bridgeInstances[0]; expect(bridge.ctorArgs[0]).toBe(fakeClient); @@ -167,7 +169,7 @@ describe("createAppBridgeFactory", () => { getClient: () => fakeClient, readResource, }); - await factory(makeIframe(), tool); + await factory(makeIframe(), { kind: "tool", tool }); const bridge = bridgeInstances[0]; bridge.emit("sandboxready"); @@ -215,7 +217,7 @@ describe("createAppBridgeFactory", () => { getClient: () => fakeClient, readResource, }); - await factory(makeIframe(), tool); + await factory(makeIframe(), { kind: "tool", tool }); bridgeInstances[0].emit("sandboxready"); await flush(); expect(HOST_CAPABILITIES.sandbox).toBeUndefined(); @@ -235,7 +237,7 @@ describe("createAppBridgeFactory", () => { getClient: () => fakeClient, readResource, }); - await factory(makeIframe(), tool); + await factory(makeIframe(), { kind: "tool", tool }); const bridge = bridgeInstances[0]; bridge.emit("sandboxready"); await flush(); @@ -256,8 +258,8 @@ describe("createAppBridgeFactory", () => { readResource, }); await factory(makeIframe(), { - name: "plain", - inputSchema: { type: "object" }, + kind: "tool", + tool: { name: "plain", inputSchema: { type: "object" } }, }); bridgeInstances[0].emit("sandboxready"); await flush(); @@ -274,7 +276,7 @@ describe("createAppBridgeFactory", () => { readResource, onResourceError, }); - await factory(makeIframe(), tool); + await factory(makeIframe(), { kind: "tool", tool }); const bridge = bridgeInstances[0]; bridge.emit("sandboxready"); await flush(); @@ -295,7 +297,7 @@ describe("createAppBridgeFactory", () => { readResource, onResourceError, }); - await factory(makeIframe(), tool); + await factory(makeIframe(), { kind: "tool", tool }); const bridge = bridgeInstances[0]; bridge.emit("sandboxready"); await flush(); @@ -317,7 +319,7 @@ describe("createAppBridgeFactory", () => { readResource, onResourceError, }); - await factory(makeIframe(), tool); + await factory(makeIframe(), { kind: "tool", tool }); const bridge = bridgeInstances[0]; bridge.emit("sandboxready"); await flush(); @@ -335,7 +337,7 @@ describe("createAppBridgeFactory", () => { getClient: () => fakeClient, readResource, }); - await factory(makeIframe(), tool); + await factory(makeIframe(), { kind: "tool", tool }); const bridge = bridgeInstances[0]; bridge.emit("sandboxready"); await flush(); @@ -352,7 +354,7 @@ describe("createAppBridgeFactory", () => { getClient: () => fakeClient, readResource: vi.fn().mockResolvedValue(uiResource("

x

")), }); - await factory(makeIframe(), tool); + await factory(makeIframe(), { kind: "tool", tool }); const bridge = bridgeInstances[0]; await expect( @@ -376,7 +378,7 @@ describe("createAppBridgeFactory", () => { getClient: () => fakeClient, readResource: vi.fn().mockResolvedValue(uiResource("

x

")), }); - await factory(makeIframe(), tool); + await factory(makeIframe(), { kind: "tool", tool }); expect(bridgeInstances[0].ctorArgs[2]).toMatchObject({ downloadFile: {} }); }); @@ -394,7 +396,7 @@ describe("createAppBridgeFactory", () => { getClient: () => fakeClient, readResource: vi.fn().mockResolvedValue(uiResource("

x

")), }); - await factory(makeIframe(), tool); + await factory(makeIframe(), { kind: "tool", tool }); return bridgeInstances[0]; } diff --git a/clients/web/src/components/elements/AppRenderer/createAppBridgeFactory.ts b/clients/web/src/components/elements/AppRenderer/createAppBridgeFactory.ts index 9294c51460..3aea5c1c67 100644 --- a/clients/web/src/components/elements/AppRenderer/createAppBridgeFactory.ts +++ b/clients/web/src/components/elements/AppRenderer/createAppBridgeFactory.ts @@ -8,7 +8,7 @@ import type { McpUiHostCapabilities, McpUiResourceMeta, } from "@modelcontextprotocol/ext-apps/app-bridge"; -import type { Client } from "@modelcontextprotocol/client"; +import type { Client, Transport } from "@modelcontextprotocol/client"; import type { EmbeddedResource, Implementation, @@ -26,7 +26,21 @@ import { isHttpUrl, } from "../../../lib/downloadFile"; import { snapshotHostContext } from "./hostContext"; -import type { BridgeFactory } from "./AppRenderer"; +import { observeAppCapabilities } from "./appCapabilities"; +import type { AppRenderSource, BridgeFactory } from "./AppRenderer"; + +/** + * The `ui://` resource a render source loads, or `undefined` for an App tool + * whose `_meta` names none (in which case there is nothing to push into the + * sandbox and the frame stays empty). + */ +function resolveSourceUri(source: AppRenderSource): string | undefined { + return source.kind === "resource" + ? source.resourceUri + : getToolUiResourceUri( + source.tool as Parameters[0], + ); +} /** * Host identity advertised to MCP Apps during the bridge handshake. Static — @@ -74,6 +88,16 @@ export interface AppBridgeFactoryDeps { * frame; the error is also always console.error'd. */ onResourceError?: (err: Error) => void; + /** + * Advertise `hostCapabilities.elicitation` — "this host can forward a + * form-mode `elicitation/create` to the app and return its result to the + * server" (#1854). + * + * Off by default, and deliberately per-factory rather than global: an App + * *tool* frame is never handed an elicitation, so claiming the capability + * there would tell the app something untrue about what its host will do. + */ + advertiseElicitation?: boolean; } /** First text content block of a UI resource, plus its `_meta` (sandbox hints). */ @@ -198,7 +222,7 @@ function downloadResourceItem(item: EmbeddedResource | ResourceLink): boolean { export function createAppBridgeFactory( deps: AppBridgeFactoryDeps, ): BridgeFactory { - return async (iframe, tool) => { + return async (iframe, source) => { const client = deps.getClient(); if (!client) { throw new Error("Cannot render MCP App: no connected MCP client."); @@ -211,7 +235,17 @@ export function createAppBridgeFactory( // Per-app copy so the approved-sandbox echo (set on sandboxready below) // never mutates the shared HOST_CAPABILITIES constant — each app may // declare its own csp/permissions. - const hostCapabilities: McpUiHostCapabilities = { ...HOST_CAPABILITIES }; + const hostCapabilities: McpUiHostCapabilities = { + ...HOST_CAPABILITIES, + // `elicitation` is not part of ext-apps 1.7.5's `McpUiHostCapabilities` + // (ext-apps#733 adds it), so it is spread in as an extra key. The bridge + // forwards the capabilities object verbatim in its `ui/initialize` + // response, which is exactly what the app reads. TODO: drop the cast when + // a release containing #733 ships. + ...(deps.advertiseElicitation + ? ({ elicitation: {} } as Partial) + : {}), + }; // ext-apps' `AppBridge` peers on SDK v1's `Client`/`Implementation`; both // are runtime-compatible with v2's. Cast at this single construction // boundary. TODO: drop when ext-apps#702 ships a v2 peer release. @@ -234,9 +268,7 @@ export function createAppBridgeFactory( bridge.addEventListener("sandboxready", () => { void (async () => { try { - const uri = getToolUiResourceUri( - tool as Parameters[0], - ); + const uri = resolveSourceUri(source); if (!uri) return; const result = await deps.readResource(uri); const { html, meta } = extractHtmlAndMeta(result); @@ -338,6 +370,10 @@ export function createAppBridgeFactory( const transport = new PostMessageTransport(targetWindow, targetWindow); await bridge.connect(transport); + // Record the view's raw `ui/initialize` capabilities before the bridge's + // own schema strips the keys it predates (#1854). Must follow `connect`, + // which is what installs the handler this wraps. + observeAppCapabilities(bridge, transport as unknown as Transport); return bridge; }; } diff --git a/clients/web/src/components/elements/AppRenderer/requestAppElicitation.test.ts b/clients/web/src/components/elements/AppRenderer/requestAppElicitation.test.ts new file mode 100644 index 0000000000..339f491b75 --- /dev/null +++ b/clients/web/src/components/elements/AppRenderer/requestAppElicitation.test.ts @@ -0,0 +1,91 @@ +import { describe, it, expect, vi } from "vitest"; +import type { AppBridge } from "@modelcontextprotocol/ext-apps/app-bridge"; +import type { ElicitRequest } from "@modelcontextprotocol/client"; +import { + APP_ELICITATION_TIMEOUT_MS, + requestAppElicitation, +} from "./requestAppElicitation"; + +const params: ElicitRequest["params"] = { + message: "Choose an option", + requestedSchema: { + type: "object", + properties: { choice: { type: "string" } }, + required: ["choice"], + }, +}; + +function makeBridge(options: { + appCapabilities?: Record; + request?: ReturnType; +}) { + return { + getAppCapabilities: () => options.appCapabilities, + request: options.request ?? vi.fn(), + } as unknown as AppBridge; +} + +describe("requestAppElicitation (#1854)", () => { + it("sends the standard method and params through the given bridge", async () => { + const request = vi + .fn() + .mockResolvedValue({ action: "accept", content: { choice: "option-a" } }); + const bridge = makeBridge({ + appCapabilities: { elicitation: {} }, + request, + }); + + await expect(requestAppElicitation(bridge, params)).resolves.toEqual({ + action: "accept", + content: { choice: "option-a" }, + }); + // The method and params must reach the app UNCHANGED — the whole contract + // is that no custom method or result shape is introduced. + const [sent, , options] = request.mock.calls[0]; + expect(sent).toEqual({ method: "elicitation/create", params }); + expect(options).toEqual({ timeout: APP_ELICITATION_TIMEOUT_MS }); + }); + + it("honors a caller-supplied timeout", async () => { + const request = vi.fn().mockResolvedValue({ action: "cancel" }); + const bridge = makeBridge({ + appCapabilities: { elicitation: {} }, + request, + }); + await requestAppElicitation(bridge, params, 1234); + expect(request.mock.calls[0][2]).toEqual({ timeout: 1234 }); + }); + + it("fails closed when the app did not advertise elicitation", async () => { + const request = vi.fn(); + const bridge = makeBridge({ appCapabilities: {}, request }); + await expect(requestAppElicitation(bridge, params)).rejects.toThrow( + /does not support elicitation/, + ); + // Not merely "returns an error" — nothing is sent at all, so a wedged app + // cannot hold the server's request open for the full timeout. + expect(request).not.toHaveBeenCalled(); + }); + + it("fails closed when the app advertised nothing at all", async () => { + await expect(requestAppElicitation(makeBridge({}), params)).rejects.toThrow( + /does not support elicitation/, + ); + }); + + it("propagates a bridge failure so the caller can fall back", async () => { + const bridge = makeBridge({ + appCapabilities: { elicitation: {} }, + request: vi.fn().mockRejectedValue(new Error("transport closed")), + }); + await expect(requestAppElicitation(bridge, params)).rejects.toThrow( + /transport closed/, + ); + }); + + it("keeps the answer timeout far above the SDK's request default", () => { + // The thing being waited on is a person, not a server. 60s (the SDK + // default) would abandon a user who paused to think. + expect(APP_ELICITATION_TIMEOUT_MS).toBeGreaterThan(60_000); + }); +}); diff --git a/clients/web/src/components/elements/AppRenderer/requestAppElicitation.ts b/clients/web/src/components/elements/AppRenderer/requestAppElicitation.ts new file mode 100644 index 0000000000..e7907926d5 --- /dev/null +++ b/clients/web/src/components/elements/AppRenderer/requestAppElicitation.ts @@ -0,0 +1,59 @@ +import type { AppBridge } from "@modelcontextprotocol/ext-apps/app-bridge"; +import type { ElicitRequest, ElicitResult } from "@modelcontextprotocol/client"; +import { ElicitResultSchema } from "@modelcontextprotocol/core"; +import { appAdvertisesElicitation } from "./appCapabilities"; + +/** + * How long the host waits for an app to answer an elicitation before giving up + * and falling back to the native UI. + * + * Deliberately generous: unlike a tool call, the thing being waited on is a + * *person* filling in a form, and the SDK's 60s request default would abandon + * a user who paused to think. Ten minutes bounds a bridge that will never + * answer (a wedged app, a closed tab) without ever racing a real user. + */ +export const APP_ELICITATION_TIMEOUT_MS = 10 * 60 * 1000; + +/** + * Forward a form-mode `elicitation/create` to one specific running MCP App and + * return the app's standard `ElicitResult` (#1854). + * + * This is ext-apps' own `AppBridge.requestElicitation` from + * modelcontextprotocol/ext-apps#733 — same method, same params, same result — + * implemented against the bridge's generic `request()` because the released + * package (1.7.5) predates that PR. Replace the body with a call to + * `bridge.requestElicitation(params)` once a release containing #733 ships; + * nothing on the wire changes when that happens. + * + * Throwing is meaningful to every caller: it is the signal to fall back to the + * native elicitation UI. A user's `decline` or `cancel` is a *resolved* result, + * never a throw. + */ +export async function requestAppElicitation( + bridge: AppBridge, + params: ElicitRequest["params"], + timeoutMs: number = APP_ELICITATION_TIMEOUT_MS, +): Promise { + // Fail closed on the app's own advertisement rather than discovering it as a + // "-32601 method not found" ten minutes later: an app that never registered + // an elicitation handler is a fallback case, not an error case. + // NOT `bridge.getAppCapabilities()` directly: ext-apps 1.7.5 strips the + // `elicitation` key when it parses `ui/initialize`. See appCapabilities.ts. + if (!appAdvertisesElicitation(bridge)) { + throw new Error("App does not support elicitation"); + } + // ext-apps 1.7.5's send union (`AppRequest`) has no `ElicitRequest` member — + // that is precisely what #733 adds — so TypeScript sees no overlap with the + // existing members and a single `as` is rejected. The double cast is the + // documented-gap case: the runtime is a plain JSON-RPC send of the standard + // method with its standard params, verified against the app-side handler in + // the fixture and the bridge tests. Confined to this one line and removed + // with the ext-apps bump, when `bridge.requestElicitation(params)` replaces it. + const request = { + method: "elicitation/create", + params, + } as unknown as Parameters[0]; + return (await bridge.request(request, ElicitResultSchema, { + timeout: timeoutMs, + })) as ElicitResult; +} diff --git a/clients/web/src/components/screens/AppsScreen/AppsScreen.tsx b/clients/web/src/components/screens/AppsScreen/AppsScreen.tsx index deaecd374f..ab56111f04 100644 --- a/clients/web/src/components/screens/AppsScreen/AppsScreen.tsx +++ b/clients/web/src/components/screens/AppsScreen/AppsScreen.tsx @@ -662,7 +662,7 @@ export function AppsScreen({ { + it("queues a request and notifies subscribers", () => { + const controller = new AppElicitationController(); + const listener = vi.fn(); + controller.subscribe(listener); + + void request(controller, "a").catch(() => {}); + + expect(listener).toHaveBeenCalledTimes(1); + expect(controller.getEntries()).toHaveLength(1); + expect(controller.getEntries()[0]).toMatchObject({ + requestId: "a", + resourceUri: "ui://demo/pick.html", + }); + }); + + it("keeps the entries array identity stable between changes", () => { + // `useSyncExternalStore` re-renders on every getSnapshot identity change, + // so a fresh array per read would loop forever. + const controller = new AppElicitationController(); + expect(controller.getEntries()).toBe(controller.getEntries()); + }); + + it("settle resolves the render promise and drops the entry", async () => { + const controller = new AppElicitationController(); + const pending = request(controller, "a"); + controller.settle("a", { action: "accept", content: { choice: "x" } }); + await expect(pending).resolves.toEqual({ + action: "accept", + content: { choice: "x" }, + }); + expect(controller.getEntries()).toHaveLength(0); + }); + + it("fail rejects the render promise so the client falls back", async () => { + const controller = new AppElicitationController(); + const pending = request(controller, "a"); + controller.fail("a", new Error("sandbox unavailable")); + await expect(pending).rejects.toThrow(/sandbox unavailable/); + expect(controller.getEntries()).toHaveLength(0); + }); + + it("settling an unknown or already-settled id is a no-op", async () => { + const controller = new AppElicitationController(); + const pending = request(controller, "a"); + controller.settle("a", { action: "cancel" }); + await expect(pending).resolves.toEqual({ action: "cancel" }); + // A second settle must not throw — the modal's unmount and the client's + // abort can race, and both call in. + expect(() => controller.settle("a", { action: "decline" })).not.toThrow(); + expect(() => controller.fail("nope", new Error("x"))).not.toThrow(); + }); + + it("keeps concurrent requests independent", async () => { + const controller = new AppElicitationController(); + const first = request(controller, "a", "ui://demo/first.html"); + const second = request(controller, "b", "ui://demo/second.html"); + expect(controller.getEntries().map((e) => e.requestId)).toEqual(["a", "b"]); + + controller.settle("b", { action: "accept", content: { choice: "b" } }); + expect(controller.getEntries().map((e) => e.requestId)).toEqual(["a"]); + await expect(second).resolves.toMatchObject({ content: { choice: "b" } }); + + controller.settle("a", { action: "accept", content: { choice: "a" } }); + await expect(first).resolves.toMatchObject({ content: { choice: "a" } }); + }); + + it("drops and rejects an entry when the originating request aborts", async () => { + const controller = new AppElicitationController(); + const aborter = new AbortController(); + const pending = request( + controller, + "a", + "ui://demo/pick.html", + aborter.signal, + ); + expect(controller.getEntries()).toHaveLength(1); + aborter.abort(); + await expect(pending).rejects.toThrow(/aborted/); + expect(controller.getEntries()).toHaveLength(0); + }); + + it("never queues a request whose signal already aborted", async () => { + const controller = new AppElicitationController(); + const aborter = new AbortController(); + aborter.abort(); + await expect( + request(controller, "a", "ui://demo/pick.html", aborter.signal), + ).rejects.toThrow(/aborted/); + expect(controller.getEntries()).toHaveLength(0); + }); + + it("stops notifying an unsubscribed listener", () => { + const controller = new AppElicitationController(); + const listener = vi.fn(); + const unsubscribe = controller.subscribe(listener); + unsubscribe(); + void request(controller, "a").catch(() => {}); + expect(listener).not.toHaveBeenCalled(); + }); +}); diff --git a/clients/web/src/lib/appElicitationController.ts b/clients/web/src/lib/appElicitationController.ts new file mode 100644 index 0000000000..d529cee2b6 --- /dev/null +++ b/clients/web/src/lib/appElicitationController.ts @@ -0,0 +1,99 @@ +import type { ElicitResult } from "@modelcontextprotocol/client"; +import type { + AppElicitationRenderer, + AppElicitationRequest, +} from "@inspector/core/mcp/appElicitation.js"; + +/** + * One app-rendered elicitation awaiting an answer, plus the settle functions + * for the `InspectorClient` promise it belongs to (#1854). + * + * The entry — not a "currently active app" — is what owns the renderer, so two + * concurrent elicitations each drive their own iframe and bridge and cannot + * resolve through each other's. + */ +export interface AppElicitationEntry extends AppElicitationRequest { + /** Hands the app's standard `ElicitResult` back to the server. */ + resolve: (result: ElicitResult) => void; + /** Asks `InspectorClient` to fall back to the native elicitation UI. */ + reject: (error: Error) => void; +} + +/** + * Bridges `InspectorClient`'s renderer callback — supplied at construction, + * long before any React tree exists — to the React component that actually + * mounts the app. + * + * The client is given {@link render} once and for all; the UI subscribes and + * re-renders as entries come and go. Without this indirection the renderer + * would have to be rebuilt (and the client reconstructed) whenever the host + * component remounted. + */ +export class AppElicitationController { + private entries: AppElicitationEntry[] = []; + private listeners = new Set<() => void>(); + + /** Current queue. Stable identity between changes, for `useSyncExternalStore`. */ + getEntries = (): AppElicitationEntry[] => this.entries; + + subscribe = (listener: () => void): (() => void) => { + this.listeners.add(listener); + return () => { + this.listeners.delete(listener); + }; + }; + + private emit(): void { + for (const listener of this.listeners) listener(); + } + + /** + * The {@link AppElicitationRenderer} handed to `InspectorClient`. Queues the + * request for the UI and resolves when {@link settle} or {@link fail} is + * called for it. + * + * An abort of the originating request (cancelled tool call, disconnect) drops + * the entry and rejects, so a modal can never outlive the request behind it. + */ + render: AppElicitationRenderer = (request: AppElicitationRequest) => + new Promise((resolve, reject) => { + const entry: AppElicitationEntry = { ...request, resolve, reject }; + const onAbort = () => { + this.remove(entry.requestId); + reject(new Error("App-rendered elicitation aborted")); + }; + if (request.signal.aborted) { + onAbort(); + return; + } + request.signal.addEventListener("abort", onAbort, { once: true }); + this.entries = [...this.entries, entry]; + this.emit(); + }); + + private take(requestId: string): AppElicitationEntry | undefined { + const entry = this.entries.find((e) => e.requestId === requestId); + if (entry) this.remove(requestId); + return entry; + } + + private remove(requestId: string): void { + const next = this.entries.filter((e) => e.requestId !== requestId); + if (next.length === this.entries.length) return; + this.entries = next; + this.emit(); + } + + /** + * Complete an elicitation with the app's result. `decline` and `cancel` are + * completions too — they are returned to the server, not fallen back on. + */ + settle(requestId: string, result: ElicitResult): void { + this.take(requestId)?.resolve(result); + } + + /** Give up on the app and let the native elicitation UI take the request. */ + fail(requestId: string, error: Error): void { + this.take(requestId)?.reject(error); + } +} diff --git a/clients/web/src/test/core/mcp/appElicitation.test.ts b/clients/web/src/test/core/mcp/appElicitation.test.ts new file mode 100644 index 0000000000..cf433f1685 --- /dev/null +++ b/clients/web/src/test/core/mcp/appElicitation.test.ts @@ -0,0 +1,274 @@ +import { describe, it, expect } from "vitest"; +import type { + ClientCapabilities, + ElicitRequest, + ElicitResult, + ServerCapabilities, +} from "@modelcontextprotocol/client"; +import { AjvJsonSchemaValidator } from "@modelcontextprotocol/client/validators/ajv"; +import { + getElicitationUiResourceUri, + getUiClientCapability, + getUiServerCapability, + isFormElicitation, + supportsAppElicitation, + validateAppElicitResult, +} from "@inspector/core/mcp/appElicitation.js"; +import { + MCP_APP_MIME_TYPE, + UI_EXTENSION_KEY, +} from "@inspector/core/mcp/extensions.js"; + +/** A client that satisfies all three client-side negotiation gates. */ +function eligibleClient(): ClientCapabilities { + return { + elicitation: { form: {} }, + extensions: { + [UI_EXTENSION_KEY]: { + mimeTypes: [MCP_APP_MIME_TYPE], + elicitation: {}, + }, + }, + }; +} + +/** A server that advertises the nested MCP Apps elicitation setting. */ +function eligibleServer(): ServerCapabilities { + return { extensions: { [UI_EXTENSION_KEY]: { elicitation: {} } } }; +} + +function formParams( + extra: Partial = {}, +): ElicitRequest["params"] { + return { + message: "Choose an option", + requestedSchema: { + type: "object", + properties: { choice: { type: "string" } }, + required: ["choice"], + }, + ...extra, + } as ElicitRequest["params"]; +} + +describe("appElicitation negotiation (#1854)", () => { + describe("capability readers", () => { + it("reads the UI block from either side", () => { + expect(getUiClientCapability(eligibleClient())).toEqual({ + mimeTypes: [MCP_APP_MIME_TYPE], + elicitation: {}, + }); + expect(getUiServerCapability(eligibleServer())).toEqual({ + elicitation: {}, + }); + }); + + it("returns undefined for absent, null and non-object capabilities", () => { + expect(getUiClientCapability(undefined)).toBeUndefined(); + expect(getUiClientCapability(null)).toBeUndefined(); + expect(getUiClientCapability({})).toBeUndefined(); + expect(getUiServerCapability({ extensions: {} })).toBeUndefined(); + expect( + getUiClientCapability({ + extensions: { [UI_EXTENSION_KEY]: null }, + } as unknown as ClientCapabilities), + ).toBeUndefined(); + }); + }); + + describe("supportsAppElicitation", () => { + it("is true only when all four gates pass", () => { + expect(supportsAppElicitation(eligibleClient(), eligibleServer())).toBe( + true, + ); + }); + + it("is false without the core form elicitation capability", () => { + const client = eligibleClient(); + delete client.elicitation; + expect(supportsAppElicitation(client, eligibleServer())).toBe(false); + }); + + it("is false when the client advertised only url-mode elicitation", () => { + const client = { ...eligibleClient(), elicitation: { url: {} } }; + expect(supportsAppElicitation(client, eligibleServer())).toBe(false); + }); + + it("is false when the client does not accept the MCP App MIME type", () => { + const client: ClientCapabilities = { + elicitation: { form: {} }, + extensions: { + [UI_EXTENSION_KEY]: { mimeTypes: ["text/html"], elicitation: {} }, + }, + }; + expect(supportsAppElicitation(client, eligibleServer())).toBe(false); + }); + + it("is false when the client advertises the MIME type but not elicitation", () => { + // The specific "MIME type alone is not sufficient" case: this is exactly + // what a CLI/TUI client looks like, and it must not be offered an app. + const client: ClientCapabilities = { + elicitation: { form: {} }, + extensions: { + [UI_EXTENSION_KEY]: { mimeTypes: [MCP_APP_MIME_TYPE] }, + }, + }; + expect(supportsAppElicitation(client, eligibleServer())).toBe(false); + }); + + it("is false when the server did not advertise it", () => { + expect(supportsAppElicitation(eligibleClient(), {})).toBe(false); + expect( + supportsAppElicitation(eligibleClient(), { + extensions: { [UI_EXTENSION_KEY]: {} }, + }), + ).toBe(false); + expect(supportsAppElicitation(eligibleClient(), undefined)).toBe(false); + }); + }); + + describe("getElicitationUiResourceUri", () => { + it("returns the URI from _meta.ui.resourceUri", () => { + expect( + getElicitationUiResourceUri( + formParams({ _meta: { ui: { resourceUri: "ui://demo/pick.html" } } }), + ), + ).toBe("ui://demo/pick.html"); + }); + + it("returns undefined when no app is attached", () => { + expect(getElicitationUiResourceUri(formParams())).toBeUndefined(); + expect( + getElicitationUiResourceUri(formParams({ _meta: {} })), + ).toBeUndefined(); + expect( + getElicitationUiResourceUri(formParams({ _meta: { ui: "nope" } })), + ).toBeUndefined(); + expect( + getElicitationUiResourceUri(formParams({ _meta: { ui: null } })), + ).toBeUndefined(); + expect( + getElicitationUiResourceUri(formParams({ _meta: { ui: {} } })), + ).toBeUndefined(); + }); + + it("throws on a non-string resourceUri", () => { + expect(() => + getElicitationUiResourceUri( + formParams({ _meta: { ui: { resourceUri: 42 } } }), + ), + ).toThrow(/must be a string/); + }); + + it.each([ + ["relative", "demo/pick.html"], + ["wrong scheme", "https://example.com/pick.html"], + ["scheme with no host", "ui:///pick.html"], + ["bare scheme", "ui:pick.html"], + ["not a URL at all", " "], + ])("throws on a %s URI", (_label, uri) => { + expect(() => + getElicitationUiResourceUri( + formParams({ _meta: { ui: { resourceUri: uri } } }), + ), + ).toThrow(/absolute ui:\/\/ URI/); + }); + }); + + describe("isFormElicitation", () => { + it("treats an omitted mode as form", () => { + expect(isFormElicitation(formParams())).toBe(true); + expect(isFormElicitation(formParams({ mode: "form" }))).toBe(true); + }); + + it("rejects url mode", () => { + expect(isFormElicitation(formParams({ mode: "url" }))).toBe(false); + }); + }); + + describe("validateAppElicitResult", () => { + const provider = new AjvJsonSchemaValidator(); + const params = formParams(); + + it("accepts a well-formed accept", () => { + expect( + validateAppElicitResult(provider, params, { + action: "accept", + content: { choice: "option-a" }, + }), + ).toBeUndefined(); + }); + + it("accepts decline and cancel with no content", () => { + expect( + validateAppElicitResult(provider, params, { action: "decline" }), + ).toBeUndefined(); + expect( + validateAppElicitResult(provider, params, { action: "cancel" }), + ).toBeUndefined(); + }); + + it("rejects a non-object result", () => { + expect( + validateAppElicitResult( + provider, + params, + null as unknown as ElicitResult, + ), + ).toMatch(/non-object/); + }); + + it("rejects an unknown action", () => { + expect( + validateAppElicitResult(provider, params, { + action: "maybe", + } as unknown as ElicitResult), + ).toMatch(/unknown elicitation action/); + }); + + it("rejects an accept with no usable content", () => { + expect( + validateAppElicitResult(provider, params, { + action: "accept", + } as ElicitResult), + ).toMatch(/without a content object/); + expect( + validateAppElicitResult(provider, params, { + action: "accept", + content: [] as unknown as Record, + } as ElicitResult), + ).toMatch(/without a content object/); + }); + + it("rejects content that does not match the requested schema", () => { + expect( + validateAppElicitResult(provider, params, { + action: "accept", + content: { choice: 7 } as unknown as Record, + } as ElicitResult), + ).toMatch(/does not match the requested schema/); + }); + + it("skips validation when the request declared no usable schema", () => { + const noSchema = { message: "hi" } as ElicitRequest["params"]; + expect( + validateAppElicitResult(provider, noSchema, { + action: "accept", + content: { anything: true }, + }), + ).toBeUndefined(); + }); + + it("does not reject a result over a schema the validator cannot compile", () => { + const badSchema = formParams({ + requestedSchema: { type: "object", properties: { a: { type: 9 } } }, + } as unknown as Partial); + expect( + validateAppElicitResult(provider, badSchema, { + action: "accept", + content: { a: 1 }, + }), + ).toBeUndefined(); + }); + }); +}); diff --git a/clients/web/src/test/core/mcp/extensions.test.ts b/clients/web/src/test/core/mcp/extensions.test.ts index e027c9a3fd..5a2bbb2611 100644 --- a/clients/web/src/test/core/mcp/extensions.test.ts +++ b/clients/web/src/test/core/mcp/extensions.test.ts @@ -145,4 +145,42 @@ describe("extensions (#1738, #1740)", () => { expect(map).toEqual({ [EMA_EXTENSION_KEY]: {} }); }); }); + + describe("app-rendered elicitation opt-in (#1854)", () => { + it("does not advertise the nested elicitation setting by default", () => { + const map = buildClientExtensions({ enterpriseManaged: false }); + expect(map[UI_EXTENSION_KEY]).toEqual(UI_ADVERTISEMENT); + }); + + it("nests `elicitation` inside the UI extension when opted in", () => { + const map = buildClientExtensions({ + enterpriseManaged: false, + appElicitation: true, + }); + expect(map[UI_EXTENSION_KEY]).toEqual({ + ...UI_ADVERTISEMENT, + elicitation: {}, + }); + // A nested setting, NOT a second extension — the contract is explicit + // that no new extension id is introduced. + expect(Object.keys(map)).toEqual([TASKS_EXTENSION_KEY, UI_EXTENSION_KEY]); + }); + + it("advertises nothing when the UI extension itself is turned off", () => { + const map = buildClientExtensions({ + enterpriseManaged: false, + appElicitation: true, + advertised: { [UI_EXTENSION_KEY]: false }, + }); + expect(map).not.toHaveProperty(UI_EXTENSION_KEY); + }); + + it("does not mutate the shared registry advertisement", () => { + buildClientExtensions({ enterpriseManaged: false, appElicitation: true }); + const ui = ADVERTISABLE_EXTENSIONS.find( + (e) => e.key === UI_EXTENSION_KEY, + ); + expect(ui?.advertisement).toEqual(UI_ADVERTISEMENT); + }); + }); }); diff --git a/clients/web/src/test/core/mcp/inspectorClient-app-elicitation.test.ts b/clients/web/src/test/core/mcp/inspectorClient-app-elicitation.test.ts new file mode 100644 index 0000000000..fd12401e74 --- /dev/null +++ b/clients/web/src/test/core/mcp/inspectorClient-app-elicitation.test.ts @@ -0,0 +1,431 @@ +import { describe, it, expect, vi } from "vitest"; +import type { + ClientCapabilities, + ElicitResult, + JSONRPCMessage, + ServerCapabilities, + Transport, +} from "@modelcontextprotocol/client"; +import { InspectorClient } from "@inspector/core/mcp/inspectorClient.js"; +import type { + AppElicitationRenderer, + AppElicitationRequest, +} from "@inspector/core/mcp/appElicitation.js"; +import { + MCP_APP_MIME_TYPE, + UI_EXTENSION_KEY, +} from "@inspector/core/mcp/extensions.js"; + +/** + * Routing coverage for app-rendered form elicitations (#1854). + * + * Everything here drives the REAL inbound `elicitation/create` handler over a + * fake transport, because the whole feature is a decision made inside that + * handler: which of two user interfaces answers a server's request. Asserting + * on the reply frame — rather than on an internal — is what proves the server + * gets the app's standard `ElicitResult` unchanged. + */ + +const APP_URI = "ui://demo/choose-option.html"; + +const REQUESTED_SCHEMA = { + type: "object" as const, + properties: { choice: { type: "string" as const } }, + required: ["choice"], +}; + +/** Server capabilities advertising the nested MCP Apps elicitation setting. */ +const APP_SERVER_CAPABILITIES: ServerCapabilities = { + extensions: { [UI_EXTENSION_KEY]: { elicitation: {} } }, +}; + +class ElicitTransport implements Transport { + onmessage?: (message: JSONRPCMessage) => void; + onclose?: () => void; + onerror?: (error: Error) => void; + + private readonly waiters = new Map< + string | number, + (m: JSONRPCMessage) => void + >(); + + private readonly serverCapabilities: ServerCapabilities; + + constructor( + serverCapabilities: ServerCapabilities = APP_SERVER_CAPABILITIES, + ) { + this.serverCapabilities = serverCapabilities; + } + + async start(): Promise {} + async close(): Promise {} + + async send(message: JSONRPCMessage): Promise { + if ( + "method" in message && + message.method === "initialize" && + "id" in message + ) { + const params = message.params as { protocolVersion: string }; + this.deliver({ + jsonrpc: "2.0", + id: message.id, + result: { + protocolVersion: params.protocolVersion, + capabilities: this.serverCapabilities, + serverInfo: { name: "app-elicit-server", version: "1.0.0" }, + }, + }); + return; + } + // A reply to something we injected. + if ( + "id" in message && + message.id !== undefined && + ("result" in message || "error" in message) + ) { + this.waiters.get(message.id)?.(message); + this.waiters.delete(message.id); + } + } + + /** Send an `elicitation/create` and resolve with the client's reply frame. */ + elicit( + id: number, + params: Record, + timeoutMs = 2000, + ): Promise { + const reply = new Promise((resolve) => { + this.waiters.set(id, resolve); + }); + this.deliver({ jsonrpc: "2.0", id, method: "elicitation/create", params }); + let timer: ReturnType; + return Promise.race([ + reply, + new Promise((_, reject) => { + timer = setTimeout( + () => reject(new Error(`No reply to elicitation ${id}`)), + timeoutMs, + ); + }), + ]).finally(() => clearTimeout(timer)); + } + + private deliver(message: JSONRPCMessage): void { + this.onmessage?.(message); + } +} + +function appParams(overrides: Record = {}) { + return { + message: "Choose an option", + requestedSchema: REQUESTED_SCHEMA, + _meta: { ui: { resourceUri: APP_URI } }, + ...overrides, + }; +} + +async function connectClient(options: { + transport: ElicitTransport; + appElicitation?: AppElicitationRenderer; + elicit?: boolean | { form?: boolean; url?: boolean }; +}) { + const client = new InspectorClient( + { type: "stdio", command: "noop", args: [] }, + { + environment: { transport: () => ({ transport: options.transport }) }, + elicit: options.elicit ?? { form: true }, + ...(options.appElicitation && { appElicitation: options.appElicitation }), + }, + ); + await client.connect(); + return client; +} + +/** The `io.modelcontextprotocol/ui` block the client advertised. */ +function advertisedUi(client: InspectorClient) { + const capabilities = ( + client as unknown as { clientCapabilities: ClientCapabilities } + ).clientCapabilities; + return capabilities.extensions?.[UI_EXTENSION_KEY] as + | { mimeTypes?: string[]; elicitation?: object } + | undefined; +} + +describe("app-rendered elicitation routing (#1854)", () => { + describe("capability advertisement", () => { + it("advertises the nested elicitation setting when a renderer is supplied", async () => { + const client = await connectClient({ + transport: new ElicitTransport(), + appElicitation: async () => ({ action: "cancel" }), + }); + expect(advertisedUi(client)).toEqual({ + mimeTypes: [MCP_APP_MIME_TYPE], + elicitation: {}, + }); + await client.disconnect(); + }); + + it("does not advertise it on a client with no renderer (CLI/TUI)", async () => { + // The MIME type alone is what CLI and TUI advertise, and it must stay + // that way: they know the type but cannot host an app. + const client = await connectClient({ transport: new ElicitTransport() }); + expect(advertisedUi(client)).toEqual({ mimeTypes: [MCP_APP_MIME_TYPE] }); + await client.disconnect(); + }); + + it("omits both capabilities when form elicitation is disabled", async () => { + const client = await connectClient({ + transport: new ElicitTransport(), + appElicitation: async () => ({ action: "cancel" }), + elicit: { url: true }, + }); + const capabilities = ( + client as unknown as { clientCapabilities: ClientCapabilities } + ).clientCapabilities; + expect(capabilities.elicitation?.form).toBeUndefined(); + expect(advertisedUi(client)?.elicitation).toBeUndefined(); + await client.disconnect(); + }); + }); + + describe("negotiated happy path", () => { + it("returns the app's accept result to the server, without queueing a native request", async () => { + const transport = new ElicitTransport(); + const seen: AppElicitationRequest[] = []; + const client = await connectClient({ + transport, + appElicitation: async (request) => { + seen.push(request); + return { action: "accept", content: { choice: "option-a" } }; + }, + }); + + const reply = await transport.elicit(1, appParams()); + + expect(reply).toMatchObject({ + id: 1, + result: { action: "accept", content: { choice: "option-a" } }, + }); + // Request-scoped: the renderer is told exactly which resource and which + // params, and gets a distinct id per request. + expect(seen).toHaveLength(1); + expect(seen[0].resourceUri).toBe(APP_URI); + expect(seen[0].params.message).toBe("Choose an option"); + // The native queue was never opened — the whole point of the routing. + expect(client.getPendingElicitations()).toHaveLength(0); + await client.disconnect(); + }); + + it.each([["decline"], ["cancel"]] as const)( + "returns an explicit %s without opening the native UI", + async (action) => { + const transport = new ElicitTransport(); + const client = await connectClient({ + transport, + appElicitation: async () => ({ action }) as ElicitResult, + }); + const reply = await transport.elicit(2, appParams()); + expect(reply).toMatchObject({ id: 2, result: { action } }); + expect(client.getPendingElicitations()).toHaveLength(0); + await client.disconnect(); + }, + ); + + it("gives concurrent requests distinct ids and never crosses their results", async () => { + const transport = new ElicitTransport(); + const settle = new Map void>(); + const byUri = new Map(); + const client = await connectClient({ + transport, + appElicitation: (request) => + new Promise((resolve) => { + byUri.set(request.resourceUri, request.requestId); + settle.set(request.requestId, resolve); + }), + }); + + const first = transport.elicit( + 10, + appParams({ _meta: { ui: { resourceUri: "ui://demo/first.html" } } }), + ); + const second = transport.elicit( + 11, + appParams({ _meta: { ui: { resourceUri: "ui://demo/second.html" } } }), + ); + await vi.waitFor(() => expect(settle.size).toBe(2)); + + const firstId = byUri.get("ui://demo/first.html")!; + const secondId = byUri.get("ui://demo/second.html")!; + expect(firstId).not.toBe(secondId); + // Answer them out of order: an implementation keyed on "the active app" + // rather than on the request would hand each answer to the wrong request. + settle.get(secondId)!({ action: "accept", content: { choice: "b" } }); + settle.get(firstId)!({ action: "accept", content: { choice: "a" } }); + + expect(await first).toMatchObject({ + id: 10, + result: { content: { choice: "a" } }, + }); + expect(await second).toMatchObject({ + id: 11, + result: { content: { choice: "b" } }, + }); + await client.disconnect(); + }); + }); + + describe("native fallback", () => { + /** + * Every fallback case asserts the same shape: the renderer is not used (or + * fails), and the request lands in the native pending queue instead, which + * we then answer to keep the server from waiting. + */ + async function expectNativeFallback( + transport: ElicitTransport, + client: InspectorClient, + id: number, + params: Record, + ) { + const reply = transport.elicit(id, params); + await vi.waitFor(() => + expect(client.getPendingElicitations()).toHaveLength(1), + ); + // `respond` settles the queued request; its own send is fire-and-forget. + void client + .getPendingElicitations()[0] + .respond({ action: "accept", content: { choice: "native" } }); + expect(await reply).toMatchObject({ + id, + result: { content: { choice: "native" } }, + }); + } + + it("falls back when the server did not advertise the capability", async () => { + const transport = new ElicitTransport({}); + const renderer = vi.fn(); + const client = await connectClient({ + transport, + appElicitation: renderer as unknown as AppElicitationRenderer, + }); + await expectNativeFallback(transport, client, 20, appParams()); + expect(renderer).not.toHaveBeenCalled(); + await client.disconnect(); + }); + + it("falls back when the request names no app", async () => { + const transport = new ElicitTransport(); + const renderer = vi.fn(); + const client = await connectClient({ + transport, + appElicitation: renderer as unknown as AppElicitationRenderer, + }); + await expectNativeFallback( + transport, + client, + 21, + appParams({ _meta: undefined }), + ); + expect(renderer).not.toHaveBeenCalled(); + await client.disconnect(); + }); + + it("falls back on malformed metadata", async () => { + const transport = new ElicitTransport(); + const renderer = vi.fn(); + const client = await connectClient({ + transport, + appElicitation: renderer as unknown as AppElicitationRenderer, + }); + await expectNativeFallback( + transport, + client, + 22, + appParams({ _meta: { ui: { resourceUri: "not-a-ui-uri" } } }), + ); + expect(renderer).not.toHaveBeenCalled(); + await client.disconnect(); + }); + + it("falls back for a url-mode elicitation", async () => { + const transport = new ElicitTransport(); + const renderer = vi.fn(); + const client = await connectClient({ + transport, + appElicitation: renderer as unknown as AppElicitationRenderer, + elicit: { form: true, url: true }, + }); + // A url-mode request carries no `requestedSchema` — it is a different + // params shape, and only `form` is app-renderable. + await expectNativeFallback(transport, client, 23, { + mode: "url", + message: "Sign in to continue", + url: "https://example.com/form", + elicitationId: "url-1", + _meta: { ui: { resourceUri: APP_URI } }, + }); + expect(renderer).not.toHaveBeenCalled(); + await client.disconnect(); + }); + + it("falls back when the renderer rejects (resource/sandbox/bridge failure, timeout)", async () => { + const transport = new ElicitTransport(); + const client = await connectClient({ + transport, + appElicitation: async () => { + throw new Error("App did not initialize in time"); + }, + }); + await expectNativeFallback(transport, client, 24, appParams()); + await client.disconnect(); + }); + + it("falls back when the app returns an invalid result", async () => { + const transport = new ElicitTransport(); + const client = await connectClient({ + transport, + appElicitation: async () => + ({ action: "sure" }) as unknown as ElicitResult, + }); + await expectNativeFallback(transport, client, 25, appParams()); + await client.disconnect(); + }); + + it("falls back when accepted content fails the requested schema", async () => { + const transport = new ElicitTransport(); + const client = await connectClient({ + transport, + appElicitation: async () => ({ + action: "accept", + content: { choice: 7 } as unknown as Record, + }), + }); + await expectNativeFallback(transport, client, 26, appParams()); + await client.disconnect(); + }); + }); + + describe("teardown", () => { + it("aborts a pending app elicitation on disconnect", async () => { + const transport = new ElicitTransport(); + let aborted = false; + const client = await connectClient({ + transport, + appElicitation: (request) => + new Promise((_resolve, reject) => { + request.signal.addEventListener("abort", () => { + aborted = true; + reject(new Error("aborted")); + }); + }), + }); + void transport.elicit(30, appParams()).catch(() => {}); + await vi.waitFor(() => expect(aborted).toBe(false)); + await client.disconnect(); + await vi.waitFor(() => expect(aborted).toBe(true)); + // An aborted request must NOT resurface in the native queue: the user + // abandoned it, and the connection it belonged to is gone. + expect(client.getPendingElicitations()).toHaveLength(0); + }); + }); +}); diff --git a/clients/web/src/test/integration/mcp/appElicitation.test.ts b/clients/web/src/test/integration/mcp/appElicitation.test.ts new file mode 100644 index 0000000000..394e8efae8 --- /dev/null +++ b/clients/web/src/test/integration/mcp/appElicitation.test.ts @@ -0,0 +1,168 @@ +import { describe, it, expect, afterEach, vi } from "vitest"; +import type { ElicitResult } from "@modelcontextprotocol/client"; +import { InspectorClient } from "@inspector/core/mcp/inspectorClient.js"; +import { createTransportNode } from "@inspector/core/mcp/node/transport.js"; +import type { AppElicitationRequest } from "@inspector/core/mcp/appElicitation.js"; +import { + APP_ELICITATION_URI, + createAppElicitationResource, + createAppElicitationTool, + createTestServerHttp, + createTestServerInfo, + type TestServerHttp, +} from "@modelcontextprotocol/inspector-test-server"; + +/** + * Live coverage of app-rendered form elicitations (#1854) against the public + * fixture, over a real transport. + * + * The unit tests drive the handler with a hand-written frame; this drives the + * real `app_choose_option` tool on the real composable server, so the whole + * path is exercised: the server's `_meta.ui.resourceUri`, its advertised + * `io.modelcontextprotocol/ui.elicitation` capability, the SDK's own + * `elicitInput` serialization (which is where a dropped `_meta` would hide), + * and the Inspector's routing decision. + * + * The renderer stands in for the web client's sandbox — everything above it is + * production code. + */ +describe("app-rendered elicitation, live server (#1854)", () => { + let client: InspectorClient | null = null; + const servers: TestServerHttp[] = []; + + afterEach(async () => { + if (client) { + try { + await client.disconnect(); + } catch { + // ignore + } + client = null; + } + while (servers.length) { + try { + await servers.pop()?.stop(); + } catch { + // ignore + } + } + }); + + /** + * The fixture server. `appElicitation` is the server half of the + * negotiation; omitting it is the "not negotiated" scenario. + */ + async function startServer(appElicitation: boolean): Promise { + const started = createTestServerHttp({ + serverInfo: createTestServerInfo("app-elicit-test", "1.0.0"), + tools: [createAppElicitationTool()], + resources: [createAppElicitationResource()], + ...(appElicitation && { appElicitation }), + }); + await started.start(); + servers.push(started); + return started; + } + + /** The fixture tool definition, as the server reports it in `tools/list`. */ + async function appTool(connected: InspectorClient) { + const { tools } = await connected.listTools(); + const tool = tools.find((t) => t.name === "app_choose_option"); + if (!tool) throw new Error("app_choose_option missing from tools/list"); + return tool; + } + + async function connect( + url: string, + renderer?: (request: AppElicitationRequest) => Promise, + ): Promise { + const connected = new InspectorClient( + { type: "streamable-http", url }, + { + environment: { transport: createTransportNode }, + elicit: { form: true }, + ...(renderer && { appElicitation: renderer }), + }, + ); + await connected.connect(); + client = connected; + return connected; + } + + it("routes the server's elicitation to the app and returns its result", async () => { + const started = await startServer(true); + const seen: AppElicitationRequest[] = []; + const connected = await connect(started.url, async (request) => { + seen.push(request); + return { action: "accept", content: { choice: "option-a" } }; + }); + + const result = await connected.callTool(await appTool(connected), { + prompt: "Choose option A or B.", + }); + + // The tool echoes the ElicitResult it received, so this asserts the app's + // standard result reached the SERVER — not merely the host. + const text = JSON.stringify(result.result?.content); + expect(text).toContain('\\"action\\":\\"accept\\"'); + expect(text).toContain("option-a"); + + expect(seen).toHaveLength(1); + // The URI the SERVER named, carried on the request's own `_meta`. + expect(seen[0].resourceUri).toBe(APP_ELICITATION_URI); + expect(seen[0].params.message).toBe("Choose option A or B."); + // The native queue never opened. + expect(connected.getPendingElicitations()).toHaveLength(0); + }); + + it("returns an app decline to the server without opening the native UI", async () => { + const started = await startServer(true); + const connected = await connect(started.url, async () => ({ + action: "decline", + })); + const result = await connected.callTool(await appTool(connected), {}); + expect(JSON.stringify(result.result?.content)).toContain("decline"); + expect(connected.getPendingElicitations()).toHaveLength(0); + }); + + it("falls back to the native UI when the server did not advertise the capability", async () => { + // Same tool, same `_meta.ui.resourceUri` — only the server's advertisement + // differs. This is the over-claiming failure mode: a client that renders an + // app here would strand every user of a server that never opted in. + const started = await startServer(false); + let rendererCalls = 0; + const connected = await connect(started.url, async () => { + rendererCalls++; + return { action: "cancel" }; + }); + + const tool = await appTool(connected); + const call = connected.callTool(tool, {}); + await vi.waitFor(() => + expect(connected.getPendingElicitations()).toHaveLength(1), + ); + await connected + .getPendingElicitations()[0] + .respond({ action: "accept", content: { choice: "option-b" } }); + + const result = await call; + expect(JSON.stringify(result.result?.content)).toContain("option-b"); + expect(rendererCalls).toBe(0); + }); + + it("falls back when the client cannot host an app (no renderer)", async () => { + // The CLI/TUI shape: the server offers an app, the client never advertised + // the nested capability, so the server's request is answered natively. + const started = await startServer(true); + const connected = await connect(started.url); + const tool = await appTool(connected); + const call = connected.callTool(tool, {}); + await vi.waitFor(() => + expect(connected.getPendingElicitations()).toHaveLength(1), + ); + await connected + .getPendingElicitations()[0] + .respond({ action: "accept", content: { choice: "option-a" } }); + await expect(call).resolves.toBeDefined(); + }); +}); diff --git a/core/mcp/appElicitation.ts b/core/mcp/appElicitation.ts new file mode 100644 index 0000000000..adfff56c66 --- /dev/null +++ b/core/mcp/appElicitation.ts @@ -0,0 +1,238 @@ +import type { + ClientCapabilities, + ElicitRequest, + ElicitResult, + JsonSchemaType, + jsonSchemaValidator, + ServerCapabilities, +} from "@modelcontextprotocol/client"; +import { MCP_APP_MIME_TYPE, UI_EXTENSION_KEY } from "./extensions.js"; + +/** + * App-rendered form elicitations (#1854). + * + * A server may attach an MCP App resource to a standard `elicitation/create` + * request; a host that can run MCP Apps renders that app and returns the app's + * ordinary `ElicitResult` to the server. No second extension, no custom method, + * and no custom result shape are introduced — the only new wire surface is a + * nested `elicitation` flag on the existing `io.modelcontextprotocol/ui` + * extension on each side, plus `_meta.ui.resourceUri` on the request. + * + * The helpers here mirror the ext-apps draft (modelcontextprotocol/ext-apps#733, + * SEP-3118) so the Inspector speaks exactly the proposed protocol. They are + * declared locally only because the released `@modelcontextprotocol/ext-apps` + * (1.7.5) predates that PR and exports none of them; replace + * {@link supportsAppElicitation} / {@link getElicitationUiResourceUri} with the + * package's `/server` exports once a release containing #733 ships. + */ + +/** + * MCP Apps extension settings advertised by a client, as far as app-rendered + * elicitation cares. Mirrors ext-apps' `McpUiClientCapabilities`. + */ +export interface UiClientCapabilities { + mimeTypes?: string[]; + elicitation?: object; +} + +/** + * MCP Apps extension settings advertised by a server. Mirrors ext-apps' + * `McpUiServerCapabilities` (introduced by #733): a server sets `elicitation` + * to declare it may attach an App resource to a form elicitation. + */ +export interface UiServerCapabilities { + elicitation?: object; +} + +/** Reads the `io.modelcontextprotocol/ui` block out of either side's capabilities. */ +function uiExtension( + capabilities: { extensions?: Record } | null | undefined, +): Record | undefined { + const ext = capabilities?.extensions?.[UI_EXTENSION_KEY]; + return typeof ext === "object" && ext !== null + ? (ext as Record) + : undefined; +} + +/** The MCP Apps settings a client advertised, or `undefined`. */ +export function getUiClientCapability( + capabilities: ClientCapabilities | null | undefined, +): UiClientCapabilities | undefined { + return uiExtension(capabilities) as UiClientCapabilities | undefined; +} + +/** The MCP Apps settings a server advertised, or `undefined`. */ +export function getUiServerCapability( + capabilities: ServerCapabilities | null | undefined, +): UiServerCapabilities | undefined { + return uiExtension(capabilities) as UiServerCapabilities | undefined; +} + +/** + * Whether both peers negotiated app-rendered form elicitation. All of the + * protocol's conditions except the per-request `_meta` are checked here: + * + * 1. the client advertised core form elicitation (`elicitation.form`); + * 2. the client advertised the MCP Apps MIME type; + * 3. the client advertised the nested MCP Apps `elicitation` setting; + * 4. the server advertised the nested MCP Apps `elicitation` setting. + * + * A MIME-type match alone is deliberately not sufficient — a client that can + * render App *tools* cannot necessarily resolve an elicitation through a bridge. + */ +export function supportsAppElicitation( + clientCapabilities: ClientCapabilities | null | undefined, + serverCapabilities: ServerCapabilities | null | undefined, +): boolean { + const clientUi = getUiClientCapability(clientCapabilities); + const serverUi = getUiServerCapability(serverCapabilities); + return Boolean( + clientCapabilities?.elicitation?.form && + clientUi?.mimeTypes?.includes(MCP_APP_MIME_TYPE) && + clientUi.elicitation && + serverUi?.elicitation, + ); +} + +const ELICITATION_UI_URI_ERROR = + "Elicitation UI resourceUri must be an absolute ui:// URI"; + +/** + * Rejects anything that is not an absolute `ui://host/...` URI. Matches the + * ext-apps#733 validator: a bare scheme, a relative reference, or a non-`ui` + * scheme are all unusable and must not reach the renderer. + */ +function validateElicitationUiResourceUri(resourceUri: string): void { + let parsed: URL; + try { + parsed = new URL(resourceUri); + } catch { + throw new Error(ELICITATION_UI_URI_ERROR); + } + if ( + parsed.protocol !== "ui:" || + !/^ui:\/\//i.test(resourceUri) || + parsed.host.length === 0 + ) { + throw new Error(ELICITATION_UI_URI_ERROR); + } +} + +/** + * Reads `_meta.ui.resourceUri` off an `elicitation/create` request. + * + * Returns `undefined` when the server attached no App (the ordinary case — the + * native elicitation UI handles it). Throws when the metadata is present but + * unusable, so the caller can log the server bug and still fall back rather + * than rendering something arbitrary. + */ +export function getElicitationUiResourceUri( + params: ElicitRequest["params"], +): string | undefined { + const ui = (params._meta as { ui?: unknown } | undefined)?.ui; + if (typeof ui !== "object" || ui === null) return undefined; + const resourceUri = (ui as Record).resourceUri; + if (resourceUri === undefined) return undefined; + if (typeof resourceUri !== "string") { + throw new Error("Elicitation UI resourceUri must be a string"); + } + validateElicitationUiResourceUri(resourceUri); + return resourceUri; +} + +/** + * Only `form` mode is app-renderable; an omitted mode IS form (the mode field + * post-dates form elicitation). `url` mode keeps its existing path. + */ +export function isFormElicitation(params: ElicitRequest["params"]): boolean { + const mode = (params as { mode?: unknown }).mode; + return mode === undefined || mode === "form"; +} + +/** The three actions a completed elicitation may report. */ +const ELICIT_ACTIONS = ["accept", "decline", "cancel"] as const; + +/** + * Validates what an app returned before it is handed back to the server. + * + * Returns a human-readable reason when the value is not a usable + * `ElicitResult` — an unknown action, an `accept` with no content object, or + * content that fails the request's own `requestedSchema` — and `undefined` when + * it is fine. The value is untrusted (it came from sandboxed app code), so the + * runtime checks stand regardless of the declared type. + * + * A failure here is a fallback trigger, not a protocol error: the host drops to + * the native elicitation UI rather than sending the server something that does + * not match what it asked for. + */ +export function validateAppElicitResult( + provider: jsonSchemaValidator, + params: ElicitRequest["params"], + result: ElicitResult, +): string | undefined { + if (typeof result !== "object" || result === null) { + return "App returned a non-object elicitation result"; + } + const action = (result as { action?: unknown }).action; + if (!ELICIT_ACTIONS.includes(action as (typeof ELICIT_ACTIONS)[number])) { + return `App returned an unknown elicitation action: ${JSON.stringify(action)}`; + } + // decline / cancel carry no content — they are complete as they stand. + if (action !== "accept") return undefined; + const content = (result as { content?: unknown }).content; + if ( + typeof content !== "object" || + content === null || + Array.isArray(content) + ) { + return "App accepted the elicitation without a content object"; + } + const schema = (params as { requestedSchema?: unknown }).requestedSchema; + if (typeof schema !== "object" || schema === null) return undefined; + try { + // `requestedSchema` is the SDK's own object-schema shape and `JsonSchemaType` + // the validator provider's third-party JSON Schema interface — structurally + // compatible, nominally unrelated, exactly as in `validateToolOutput`. + const validate = provider.getValidator(schema as JsonSchemaType); + const validation = validate(content); + return validation.valid + ? undefined + : `App content does not match the requested schema: ${validation.errorMessage}`; + } catch { + // A schema the validator cannot compile is the server's problem, not the + // app's; don't reject a result over it (mirrors `validateToolOutput`). + return undefined; + } +} + +/** + * One app-rendered elicitation, scoped to the originating request. + * + * `requestId` is what makes the association request-scoped: the host keys its + * renderer/bridge by it, so two concurrent elicitations for different resource + * URIs can never resolve through each other's bridges. + */ +export interface AppElicitationRequest { + /** Unique per originating `elicitation/create` request. */ + requestId: string; + /** The validated absolute `ui://` URI the app is loaded from. */ + resourceUri: string; + /** The original request params, forwarded through the bridge unchanged. */ + params: ElicitRequest["params"]; + /** Aborts when the originating request is cancelled or the client disconnects. */ + signal: AbortSignal; +} + +/** + * Host-supplied renderer for {@link AppElicitationRequest}s. Only a client that + * can actually host MCP Apps (today: the web client, when its sandbox renderer + * is available) provides one — providing it is what opts the client into + * advertising the nested MCP Apps `elicitation` capability. + * + * Resolves with the app's standard `ElicitResult`. Rejecting is a request to + * fall back to the native elicitation UI; it must not be used to signal a user + * decision, since `decline` and `cancel` are themselves completed elicitations. + */ +export type AppElicitationRenderer = ( + request: AppElicitationRequest, +) => Promise; diff --git a/core/mcp/extensions.ts b/core/mcp/extensions.ts index db7e0e1287..e3c32f08d9 100644 --- a/core/mcp/extensions.ts +++ b/core/mcp/extensions.ts @@ -98,6 +98,19 @@ export interface BuildClientExtensionsInput { * over the registry's `defaultAdvertised`; an absent key falls back to it. */ advertised?: Record; + /** + * True when this client can render an MCP App and resolve an + * `elicitation/create` request through its bridge (#1854). Adds the nested + * `elicitation` setting to the UI extension's advertisement, which is half of + * the negotiation a server checks before attaching an App to an elicitation. + * + * Deliberately an input rather than a registry default: the shared + * `InspectorClient` knowing the MCP Apps MIME type says nothing about whether + * the *client* has a sandbox renderer, so CLI and TUI must never advertise it. + * Ignored when the UI extension itself is not advertised — a nested setting on + * an extension we did not declare would be meaningless. + */ + appElicitation?: boolean; } /** @@ -126,6 +139,14 @@ export function buildClientExtensions( : {}; } } + // Nested app-rendered-elicitation opt-in (#1854), layered onto the UI + // extension's own advertisement rather than added as a second extension. + // Guarded on the UI entry actually being present so turning the Apps + // extension off in Server Settings also turns this off. + const uiAdvertisement = map[UI_EXTENSION_KEY]; + if (input.appElicitation && uiAdvertisement) { + map[UI_EXTENSION_KEY] = { ...uiAdvertisement, elicitation: {} }; + } if (input.enterpriseManaged) { map[EMA_EXTENSION_KEY] = {}; } diff --git a/core/mcp/inspectorClient.ts b/core/mcp/inspectorClient.ts index 369a509d7b..b2b6ae88aa 100644 --- a/core/mcp/inspectorClient.ts +++ b/core/mcp/inspectorClient.ts @@ -132,6 +132,13 @@ import { type ModernDetailedTask, } from "./modernTaskSchemas.js"; import { buildClientExtensions } from "./extensions.js"; +import { + getElicitationUiResourceUri, + isFormElicitation, + supportsAppElicitation, + validateAppElicitResult, + type AppElicitationRenderer, +} from "./appElicitation.js"; import { EmptyResultSchema, CallToolResultSchema, @@ -562,6 +569,27 @@ export class InspectorClient extends InspectorClientEventTarget { // Per-extension advertise overrides (#1738); undefined key falls back to the // registry default in ADVERTISABLE_EXTENSIONS. private readonly advertisedExtensions?: Record; + /** + * Host-supplied renderer for app-rendered form elicitations (#1854), or + * undefined on a client that cannot host MCP Apps. Its presence is what + * advertises the nested MCP Apps `elicitation` capability, so this is the one + * fact both the advertisement and the routing gate read. + */ + private readonly appElicitationRenderer?: AppElicitationRenderer; + + /** + * Monotonic counter behind the request-scoped id handed to the renderer. The + * association MUST be per request (a map keyed by this id owns the resource + * URI, renderer instance and promise on the host side) so two concurrent + * elicitations cannot resolve through each other's bridges. + */ + private appElicitationSeq = 0; + /** + * Abort controllers for app-rendered elicitations still awaiting an answer. + * Aborted alongside the native pending queue on disconnect, so a rendered app + * cannot outlive the connection that asked for it. + */ + private activeAppElicitations = new Set(); private receiverTaskTtlMs: number | (() => number); private receiverTaskRecords: Map = new Map(); // OAuth support (config owned by oauthManager; client delegates and uses !!oauthManager for "is OAuth configured") @@ -616,6 +644,7 @@ export class InspectorClient extends InspectorClientEventTarget { this.elicit = options.elicit ?? true; this.receiverTasks = options.receiverTasks ?? false; this.advertisedExtensions = options.advertisedExtensions; + this.appElicitationRenderer = options.appElicitation; this.receiverTaskTtlMs = options.receiverTaskTtlMs ?? 60_000; this.progress = options.progress ?? true; this.resetTimeoutOnProgress = options.resetTimeoutOnProgress ?? true; @@ -785,6 +814,14 @@ export class InspectorClient extends InspectorClientEventTarget { const advertisedExtensions = buildClientExtensions({ enterpriseManaged: options.oauth?.enterpriseManaged ?? false, advertised: this.advertisedExtensions, + // Read off the built `capabilities.elicitation.form` rather than + // re-deriving from `options.elicit`: the nested MCP Apps `elicitation` + // setting must never be advertised without the core form capability it + // extends, and two derivations of the same fact can drift. Disabling form + // elicitation therefore drops both, as the contract requires. (#1854) + appElicitation: + this.appElicitationRenderer !== undefined && + capabilities.elicitation?.form !== undefined, }); if (Object.keys(advertisedExtensions).length > 0) { capabilities.extensions = { @@ -1688,6 +1725,13 @@ export class InspectorClient extends InspectorClientEventTarget { elicitation.cancel(); } this.pendingElicitations = []; + // App-rendered elicitations (#1854) are not in the queue above — they live + // in the host's renderer — so abort them here on the same teardown paths. + // `tryAppElicitation` removes each controller in its own `finally`. + for (const controller of this.activeAppElicitations) { + controller.abort(); + } + this.activeAppElicitations.clear(); } /** @@ -2820,11 +2864,17 @@ export class InspectorClient extends InspectorClientEventTarget { * corresponding `ElicitResult` (echoed to the server on retry); only a * genuine failure or a `signal` abort rejects. */ - private enqueuePendingElicitation( + private async enqueuePendingElicitation( request: ElicitRequest, origin: PendingRequestOrigin, signal?: AbortSignal, ): Promise { + // App-rendered form elicitation (#1854) is offered first and falls back to + // the native queue below on every failure. Both entry points — the inbound + // `elicitation/create` handler and the MRTR driver's embedded requests — + // funnel through here, so neither can miss the routing. + const appResult = await this.tryAppElicitation(request, signal); + if (appResult) return appResult; // See {@link enqueuePendingSample} — Promise settle is idempotent. return new Promise((resolvePromise, rejectPromise) => { const elicitation = new ElicitationCreateMessage( @@ -2842,6 +2892,92 @@ export class InspectorClient extends InspectorClientEventTarget { }); } + /** + * Attempt to resolve an `elicitation/create` request by rendering the MCP App + * the server attached to it (#1854), returning the app's standard + * `ElicitResult`. + * + * Returns `null` for "not app-rendered — use the native UI", which covers + * every negotiation gate and every failure mode the contract lists: either + * peer did not negotiate the capability, the metadata is absent or unusable, + * the mode is not `form`, the renderer failed (resource read, sandbox/bridge + * init, missing app capability, timeout), or the app returned something that + * is not a valid result for this request. An explicit `decline` or `cancel` + * is a *completed* elicitation and is returned, not fallen back on. + * + * The one case that does not fall back is an abort of the originating + * request: the caller is cancelling the whole elicitation, so re-opening it + * in the native queue would resurrect work the user just abandoned. + */ + private async tryAppElicitation( + request: ElicitRequest, + signal?: AbortSignal, + ): Promise { + const renderer = this.appElicitationRenderer; + if (!renderer) return null; + // Gate 1-3 (client) + 4 (server), from the negotiated capabilities of this + // connection — `this.capabilities` is populated at initialize. + if (!supportsAppElicitation(this.clientCapabilities, this.capabilities)) { + return null; + } + // Only form mode is app-renderable; `url` keeps its existing path. + if (!isFormElicitation(request.params)) return null; + let resourceUri: string | undefined; + try { + resourceUri = getElicitationUiResourceUri(request.params); + } catch (error) { + this.logger.warn( + { error }, + "Elicitation carried unusable _meta.ui.resourceUri; using the native elicitation UI", + ); + return null; + } + if (!resourceUri) return null; + + // Request-scoped abort: forwards the caller's signal (MRTR cancellation) + // and is aborted by `settleAndDropPendingPeerRequests` on disconnect, so a + // rendered app cannot outlive the connection that asked for it. + const controller = new AbortController(); + const forwardAbort = () => controller.abort(signal?.reason); + if (signal) { + if (signal.aborted) forwardAbort(); + else signal.addEventListener("abort", forwardAbort, { once: true }); + } + this.activeAppElicitations.add(controller); + try { + const result = await renderer({ + requestId: `app-elicitation-${++this.appElicitationSeq}`, + resourceUri, + params: request.params, + signal: controller.signal, + }); + this.outputValidator ??= new AjvJsonSchemaValidator(); + const invalid = validateAppElicitResult( + this.outputValidator, + request.params, + result, + ); + if (invalid) { + this.logger.warn( + { resourceUri, reason: invalid }, + "App-rendered elicitation returned an invalid result; using the native elicitation UI", + ); + return null; + } + return result; + } catch (error) { + if (controller.signal.aborted) throw createPendingAbortError(); + this.logger.warn( + { error, resourceUri }, + "App-rendered elicitation failed; using the native elicitation UI", + ); + return null; + } finally { + this.activeAppElicitations.delete(controller); + signal?.removeEventListener("abort", forwardAbort); + } + } + /** * Reject a still-pending request when `signal` aborts (e.g. the user cancels * the tool call while its MRTR round is awaiting an answer). No-op when diff --git a/core/mcp/types.ts b/core/mcp/types.ts index 3b92eb2c89..79475bc4fc 100644 --- a/core/mcp/types.ts +++ b/core/mcp/types.ts @@ -23,6 +23,7 @@ import type { Client } from "@modelcontextprotocol/client"; import type { OAuthClientProvider } from "@modelcontextprotocol/client"; import type { Transport } from "@modelcontextprotocol/client"; import type { InspectorLogger } from "../logging/logger.js"; +import type { AppElicitationRenderer } from "./appElicitation.js"; import type { JsonValue } from "../json/jsonUtils.js"; import type { ClientConfig, @@ -1025,6 +1026,21 @@ export interface InspectorClientOptions { */ advertisedExtensions?: Record; + /** + * Renders an app-rendered form elicitation (#1854) and resolves with the + * app's standard `ElicitResult`. + * + * Supplying this is what opts a client into advertising the nested MCP Apps + * `elicitation` capability — so only a client that can actually host an MCP + * App and drive its bridge should pass one (today: the web client, when the + * sandbox renderer is available). CLI and TUI pass nothing and therefore + * never claim the capability, even though they share this client. + * + * A rejection means "fall back to the native elicitation UI"; a resolved + * `decline`/`cancel` is a completed elicitation and is returned to the server. + */ + appElicitation?: AppElicitationRenderer; + /** * Whether to enable listChanged notification handlers (default: true) * If enabled, InspectorClient will subscribe to list_changed notifications and fire diff --git a/package.json b/package.json index 11ddd4785b..6d1ee7f8aa 100644 --- a/package.json +++ b/package.json @@ -65,12 +65,13 @@ "coverage:tui": "cd clients/tui && npm run test:coverage", "coverage:web": "cd clients/web && npm run test:coverage", "coverage:launcher": "cd clients/launcher && npm run test:coverage", - "smoke": "npm run smoke:launcher && npm run smoke:cli && npm run smoke:tui && npm run smoke:web && npm run smoke:web:browser && npm run smoke:web:app", + "smoke": "npm run smoke:launcher && npm run smoke:cli && npm run smoke:tui && npm run smoke:web && npm run smoke:web:browser && npm run smoke:web:app && npm run smoke:web:elicit", "smoke:cli": "node scripts/smoke-cli.mjs", "smoke:tui": "node scripts/smoke-tui.mjs", "smoke:web": "node scripts/smoke-web.mjs", "smoke:web:browser": "cd clients/web && npx playwright install chromium && node ../../scripts/smoke-web-browser.mjs", "smoke:web:app": "cd clients/web && npx playwright install chromium && node ../../scripts/smoke-web-app.mjs", + "smoke:web:elicit": "cd clients/web && npx playwright install chromium && node ../../scripts/smoke-web-elicitation.mjs", "smoke:launcher": "node scripts/smoke-launcher.mjs", "pack:verify": "node scripts/pack-and-verify.mjs", "prepack": "npm run build", diff --git a/scripts/smoke-web-elicitation.mjs b/scripts/smoke-web-elicitation.mjs new file mode 100644 index 0000000000..16cf2bd91c --- /dev/null +++ b/scripts/smoke-web-elicitation.mjs @@ -0,0 +1,318 @@ +#!/usr/bin/env node +/** + * Headless-browser smoke for app-rendered form elicitations (#1854). + * + * `smoke:web:app` proves an App *tool* renders. This proves the other thing an + * MCP App can now do: answer a server's `elicitation/create`. It drives the + * whole negotiated chain end to end against the public fixture — + * **connect → call the tool → server elicits → app renders in the sandbox → + * user clicks → the app's standard `ElicitResult` reaches the server** — and + * then drives the SAME tool against a server that did NOT advertise the nested + * MCP Apps `elicitation` capability, which must fall back to the Inspector's + * native elicitation form. + * + * The fallback half is the more valuable of the two. The failure mode this + * feature can produce is not "the app doesn't render" (loud, obvious) but + * "an app renders when it should not have been offered one" — a client that + * over-claims the capability strands every user of a server that never opted + * in. Asserting the native form appears is what pins that. + * + * Two nested frames matter here: the outer trusted sandbox-proxy iframe and the + * inner sandboxed iframe holding the untrusted app. Clicking the app's button + * therefore needs `frameLocator(...).frameLocator(...)`, not one hop. + * + * Set `SMOKE_SCREENSHOT_DIR` to capture PNGs of each state (used to attach + * proof to a PR); unset, it asserts only. Playwright is resolved with a + * `createRequire` based at clients/web/package.json for the reason documented at + * length in smoke-web-browser.mjs. + * + * Expects `clients/web/dist` and `clients/launcher/build` to be built first. + * `test-servers/build` is built on demand if missing, as in smoke:web:app. + */ + +import { spawn, spawnSync } from "node:child_process"; +import { existsSync, mkdirSync } from "node:fs"; +import { createRequire } from "node:module"; +import { setTimeout as delay } from "node:timers/promises"; +import { join, resolve } from "node:path"; +import { startProdWebServer } from "./lib/prod-web-server.mjs"; +import { stopChild } from "./lib/child-cleanup.mjs"; +import { resolveNodeBin } from "./lib/resolve-node-bin.mjs"; + +const repoRoot = resolve(import.meta.dirname, ".."); +const requireFromWeb = createRequire( + resolve(repoRoot, "clients/web/package.json"), +); + +const composableServer = join( + repoRoot, + "test-servers", + "build", + "server-composable.js", +); +const configPath = (name) => + join(repoRoot, "test-servers", "configs", `${name}.json`); + +const HOST = "127.0.0.1"; +// Distinct from the other web smokes (6299 / 6298 / 6297) so a prior run whose +// port is still in TIME_WAIT can't EADDRINUSE this one. +const PORT = process.env.SMOKE_WEB_ELICIT_PORT ?? "6296"; +const TOKEN = "smoke-web-elicit-token"; +const TOOL = "app_choose_option"; +const SHOT_DIR = process.env.SMOKE_SCREENSHOT_DIR; + +const servers = []; +let browser = null; +const web = startProdWebServer({ + host: HOST, + port: PORT, + token: TOKEN, + label: "smoke:web:elicit", +}); + +async function shutdown() { + if (browser) { + try { + await browser.close(); + } catch { + // best-effort + } + browser = null; + } + await web.stop(); + while (servers.length) { + await stopChild(servers.pop(), { + label: "smoke:web:elicit", + what: "MCP test server", + }); + } +} + +async function fail(message) { + console.error(`smoke:web:elicit FAILED — ${message}`); + await shutdown(); + process.exit(1); +} + +/** Build the composable test server bundle if it isn't present yet. */ +function ensureTestServer() { + if (existsSync(composableServer)) return; + console.log( + "smoke:web:elicit — building test-servers (missing build output)...", + ); + const r = spawnSync( + process.execPath, + [ + resolveNodeBin("typescript", "tsc", repoRoot), + "-p", + "test-servers", + "--noCheck", + ], + { cwd: repoRoot, stdio: "inherit" }, + ); + if (r.status !== 0 || !existsSync(composableServer)) { + throw new Error( + "could not build the test servers (test-servers/build/server-composable.js). " + + "Run `npm run test-servers:build` from clients/web.", + ); + } +} + +/** + * Spawn a composable test server and wait for the URL it announces. + * + * The announced line is authoritative: `createTestServerHttp` resolves its port + * with `findAvailablePort()`, which walks upward when the configured one is + * taken. Both stdio channels are scanned because the announcement goes to + * stderr (`console.error` in server-composable.ts). + */ +async function startMcpServer(configName) { + const child = spawn( + process.execPath, + [composableServer, "--config", configPath(configName)], + { cwd: repoRoot, stdio: ["ignore", "pipe", "pipe"] }, + ); + servers.push(child); + let out = ""; + child.stdout.on("data", (d) => (out += d)); + child.stderr.on("data", (d) => (out += d)); + let exited = false; + let spawnError = null; + child.on("error", (err) => (spawnError = err)); + child.on("exit", () => (exited = true)); + child.on("close", () => (exited = true)); + + for (let attempt = 0; attempt < 120; attempt++) { + const announced = out.match(/listening at (http:\/\/\S+)/i); + if (announced) return announced[1]; + if (spawnError) { + throw new Error( + `could not spawn the MCP test server (${composableServer}): ${spawnError.message}`, + ); + } + if (exited) throw new Error(`MCP test server exited early:\n${out}`); + await delay(250); + } + throw new Error(`MCP test server did not start within 30s:\n${out}`); +} + +async function loadChromium() { + let chromium; + try { + ({ chromium } = requireFromWeb("playwright")); + } catch (err) { + throw new Error( + `could not resolve the Playwright package from clients/web — run \`npm install\` at the repo root (${err instanceof Error ? err.message : String(err)})`, + ); + } + try { + return await chromium.launch({ headless: true }); + } catch (err) { + throw new Error( + `chromium failed to launch — on a bare Linux box run \`npx playwright install --with-deps chromium\` for the system libraries (${err instanceof Error ? err.message : String(err)})`, + ); + } +} + +async function shot(page, name) { + if (!SHOT_DIR) return; + mkdirSync(SHOT_DIR, { recursive: true }); + await page.screenshot({ + path: join(SHOT_DIR, `${name}.png`), + fullPage: false, + }); + console.log(`smoke:web:elicit — captured ${name}.png`); +} + +/** Connect to `mcpUrl` through the deep link and wait for the Tools list. */ +async function connect(page, mcpUrl) { + const url = + `${web.baseUrl}/?serverUrl=${encodeURIComponent(mcpUrl)}` + + `&transport=http&autoConnect=${TOKEN}`; + const response = await page.goto(url, { + waitUntil: "domcontentloaded", + timeout: 30_000, + }); + if (!response || !response.ok()) { + throw new Error( + `GET / returned HTTP ${response ? response.status() : "no response"}`, + ); + } + const status = page.locator('[data-testid="connection-status"]'); + await status.waitFor({ state: "attached", timeout: 30_000 }); + const deeplink = await status.getAttribute("data-deeplink"); + if (deeplink !== "parsed") { + throw new Error( + `deep link was not accepted (data-deeplink="${deeplink}") — expected "parsed"`, + ); + } + await page + .locator('[data-testid="connection-status"][data-status="connected"]') + .waitFor({ state: "attached", timeout: 45_000 }); +} + +/** Select the elicitation tool in the Tools tab and run it. */ +async function runTool(page) { + // The main-view tabs are a Mantine SegmentedControl: a visually-hidden radio + // plus a sibling