diff --git a/AGENTS.md b/AGENTS.md index 0771175ec8..9dc52b66a3 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 @@ -737,13 +747,15 @@ The ⚠️ option-deletion hazard, the snapshot rule, and the recovery recipe ab - The one CLI nuance: `clients/cli`'s out-of-process `e2e.test.ts` spawns the built binary, so its `test` **builds first** via `pretest` (`test-servers:build && build`). To avoid building it twice, `clients/cli`'s `validate` folds that in — it is `format:check && lint && typecheck && test` with **no** separate `build` step (the other clients, whose tests don't spawn their bundle, keep an explicit `build`). `validate:web`/`validate:tui`/`validate:launcher` are the uniform `format:check && lint && (typecheck &&) build && test`. (#1778, #1789, #1792) `clients/web`'s `format`/`format:check` covers `src`, `server`, `.storybook`, and its top-level configs (the uniform `*.{ts,tsx,mts,cts,js,jsx,mjs,cjs}` glob — `vite.config.ts`, `tsup.runner.config.ts`, `eslint.config.js`, …), not just `src`, so the Node backend, Storybook config, and Vite/build config are prettier-gated too; `clients/launcher`'s covers `src`, `__tests__`, `scripts`, and its top-level configs (the `*.` top-level glob is non-recursive, so each nested dir — `.storybook`, `scripts` — is named explicitly). The `verify:format-coverage` guard (#1792) enforces that this coverage stays complete. - **One version per install-crossing dependency (#1896).** Because v2 is not a workspace, the root and each `clients/*` carry their own `node_modules` — and a client's `tsconfig.test.json` compiles first-party sources that live *outside* the client (`test-servers/src`, `core/`), which resolve their dependencies from the **root** install while the client's own sources resolve from the client install. So the same package can appear **twice in one `tsc` program**. At the same version that duplication is harmless; on a skew, TypeScript must relate two structurally-distinct declarations of the same type. For a deeply recursive-generic surface that is exponential: zod `4.3.6` (root) against zod `4.4.3` (`clients/web`) made `clients/web`'s `tsc -b` exhaust the 4GB default heap outright via `TS2589 Type instantiation is excessively deep`, because every `@modelcontextprotocol/*` schema is built out of zod generics. **Raising the heap with `--max-old-space-size` hides this class rather than fixing it — align the versions instead.** `npm run verify:dep-lockstep` (`scripts/verify-dep-lockstep.mjs`, in `validate`) is the durable guard: it **derives** the candidate set from **what actually enters each `tsc` program** (#1965) — every client tsconfig project is listed with `tsc --listFilesOnly` through the shared `scripts/lib/tsc-program.mjs` helper (the same machinery `verify:typecheck-coverage` reads a program through, so the two guards can't disagree about what one contains), each resolved `node_modules` file is mapped to its owning install and package, and a package reaching **one** program from **two** installs is a candidate. That is precisely the set that can put two structurally-distinct copies of a type in front of one checker — and it needs no editing when a new dependency arrives. It replaced a derivation that read the packages the shared sources named *directly*, which could not see one whose declarations arrive only through another package's `.d.ts`: `@modelcontextprotocol/sdk` is never written in first-party code (the shared sources import the split `@modelcontextprotocol/client|core|…`) yet 16 of its `.d.ts` files land in `clients/web`'s test program, so a second copy under `clients/web/node_modules` skewed unseen. Two properties are worth knowing: a package present in two installs but reached from only one in a given program is correctly **not** a candidate, and TypeScript's package-identity redirect collapses two copies at the *same* name@version (so an aligned package's own transitive dependencies load once) — the redirect stops applying the moment they skew, which is exactly when the guard needs to see both. Versions still come from the committed lockfiles, but from the entry for the **exact install path the program resolved** (`node_modules/zod`, `node_modules/a/node_modules/zod`) rather than from the install's top-level entry: a *nested* duplicate inside one install is still not a candidate on its own — folding it onto its outermost install is what keeps the set small — but once a program has loaded one, pricing it from a top-level entry that may be absent or differently versioned would let a real pair pass (Copilot). Only the installs that actually **met in one program** are compared, so a third install's copy that no program loads beside another is not evidence of anything. Any co-occurrence whose copies disagree fails, **deny-by-default**; a resolved copy with no lockfile entry fails too, since the tree and the lockfile then disagree about what was loaded. The escape hatch is `TOLERATED_SKEW` in that file, an allowlist of *names* (not version pairs, so an ordinary patch float doesn't churn it), each entry carrying why that package's types can't blow up; it is **empty today** — the four names it used to carry (`react`, `hono`, `jose`, `@modelcontextprotocol/ext-apps`) were admitted under the old derivation and none is a candidate under this one, so each would be a rationale for a skew that cannot occur. **Being listed is not a blanket exemption** — it tolerates skew only *within a major version*, since a rationale about patch-level differences says nothing about a React 18-vs-19 split, where the type surface itself changes; a cross-major skew fails even for a listed package. **When bumping a dependency that the shared sources pull in, bump it in every install that declares it** — that's the root plus whichever clients list it, not all four unconditionally (launcher declares no zod, for instance, and a package absent from an install can't skew, so the guard ignores it there). Don't add a dependency to a client just to satisfy this. Its pure helpers are unit-tested via `test:scripts` (its own, plus `scripts/lib/tsc-program.test.mjs` for the shared derivation), and it vouches — with `verify:format-coverage` and `verify:typecheck-coverage` — that its siblings are still wired into `validate`. It runs the same `tsc --listFilesOnly` pass its sibling does, in its own process, costing ~14s: the listing is deliberately **not** cached to disk between the two, because a fingerprint that missed an input would make a guard measure a program that no longer exists and pass on a real miss. - **`npm run coverage`** is the per-file ≥90 gate and is now part of `npm run ci` — never treat it as optional before a push. It supersedes the old standalone `test:integration` step: web's `test:coverage` runs the `unit` **and** `integration` projects under v8 instrumentation, so `coverage` both enforces the ≥90 gate and exercises the same web integration paths CI covers. -- **`smoke` is NOT part of `validate`** — it is included in `npm run ci`. It runs `smoke:launcher` (`--help` dispatch) plus the prod `smoke:cli` / `smoke:tui` / `smoke:web` / `smoke:web:browser` / `smoke:web:app`, and contains **no build commands** — it assumes the cli/tui/launcher bundles already exist (a full `validate` builds them; `smoke:web` builds `clients/web/dist` on demand). CI runs `validate`, then the `coverage` gate (which also covers the web integration project), then `verify:build-gate` (the #1769 build gate — see below), then `smoke` (with Playwright chromium installed just before it, since `smoke:web:browser` needs it). GitHub CI runs this same chain as separate workflow steps, with the Storybook play-function tests last (see below). +- **`smoke` is NOT part of `validate`** — it is included in `npm run ci`. It runs `smoke:launcher` (`--help` dispatch) plus the prod `smoke:cli` / `smoke:tui` / `smoke:web` / `smoke:web:browser` / `smoke:web:app` / `smoke:web:elicit`, and contains **no build commands** — it assumes the cli/tui/launcher bundles already exist (a full `validate` builds them; `smoke:web` builds `clients/web/dist` on demand). CI runs `validate`, then the `coverage` gate (which also covers the web integration project), then `verify:build-gate` (the #1769 build gate — see below), then `smoke` (with Playwright chromium installed just before it, since `smoke:web:browser` needs it). GitHub CI runs this same chain as separate workflow steps, with the Storybook play-function tests last (see below). - `smoke:launcher` (`scripts/smoke-launcher.mjs`) runs the built launcher with `--help`, `--cli --help`, and `--tui --help`, asserting each exits 0 and prints that mode's usage banner (which also proves the launcher resolved and loaded the right client build). It's the cheap dispatch check before the heavier prod smokes below. -- `smoke:web` (`scripts/smoke-web.mjs`) starts `mcp-inspector --web` (prod, no `--dev`) against the built `clients/web/dist` and asserts `GET /` serves the SPA (HTTP 200) with the injected `__INSPECTOR_API_TOKEN__`. Prod `--web` serves from `clients/web/dist`, which ships in the published package but is absent in a fresh checkout — the runner builds it on demand (`build:client` = `vite build`) on first launch, or exits with an actionable error if that build can't run (see `clients/web/server/ensure-web-build.ts` and the launcher README). `--dev` runs Vite directly and never needs `dist`. It shares the spawn/readiness/teardown helper (`scripts/lib/prod-web-server.mjs`) with **`smoke:web:browser` and `smoke:web:app`**, so the three can't drift. +- `smoke:web` (`scripts/smoke-web.mjs`) starts `mcp-inspector --web` (prod, no `--dev`) against the built `clients/web/dist` and asserts `GET /` serves the SPA (HTTP 200) with the injected `__INSPECTOR_API_TOKEN__`. Prod `--web` serves from `clients/web/dist`, which ships in the published package but is absent in a fresh checkout — the runner builds it on demand (`build:client` = `vite build`) on first launch, or exits with an actionable error if that build can't run (see `clients/web/server/ensure-web-build.ts` and the launcher README). `--dev` runs Vite directly and never needs `dist`. It shares the spawn/readiness/teardown helper (`scripts/lib/prod-web-server.mjs`) with **`smoke:web:browser`, `smoke:web:app` and `smoke:web:elicit`**, so the four can't drift. **Every web smoke runs against a throwaway catalog (#1977).** The helper mints a temp dir per run and passes it as `MCP_CATALOG_PATH`; without it the web backend falls back to the developer's real `~/.mcp-inspector/mcp.json`, which made these smokes both destructive and non-deterministic — `smoke:web:app`'s deep link persists a `deep-link` server row, so a *second* run found it already on disk, raced hydration, and drew a spurious (swallowed, non-fatal) 409 that was really just residue from the previous run. CI never saw it: a fresh `HOME` per run made every CI run look like a first run. This matches `smoke:cli` / `smoke:tui`, which have always driven a temp `--catalog`. Only the **catalog** is redirected — other per-user state under `~/.mcp-inspector` (OAuth tokens, `storage/`) stays shared, because isolating it means redirecting `HOME` wholesale, which would also move the npx and Playwright caches these smokes depend on. Teardown uses **both** halves of `scripts/lib/child-cleanup.mjs` (`stopChild` to await the child's exit, then `removeSafe` to delete the dir) — a bare `kill()` only *delivers* the signal, so removing synchronously re-enters the #1801 ENOTEMPTY race. That makes `stop()` **async**, so every caller must `await` it (and a caller's own `fail()`/`shutdown()` becomes async in turn, or execution runs past the intended exit). The isolation contract is unit-tested in `scripts/lib/prod-web-server.test.mjs` via `test:scripts`, since the smokes exit immediately after teardown and so cannot detect a regression that silently reshared the catalog or stopped cleaning up: `createTempCatalog` and `buildWebServerEnv` cover *which* catalog the server gets, and `teardownWebServer` — extracted from `stop()` for exactly this reason — is driven against a stand-in child process so the teardown asserts on the real directory rather than a spy. - `smoke:web:browser` (`scripts/smoke-web-browser.mjs`, #1615) goes a step further than `smoke:web`: it boots the same prod `--web` server and then actually **runs** the bundle in headless Chromium (Playwright — already a `clients/web` devDependency for the Storybook tests), asserting the app renders its first meaningful frame (the "Add Servers" control) with **no uncaught error**. `smoke:web` only checks the served HTML, so a Node built-in reaching the browser bundle slipped through it; this smoke catches that regression as a _class_ (e.g. #1612). The mechanism is the uncaught error, not a magic string: under Vite the excluded module becomes an empty stub and the first _call_ into it (e.g. `fs.readFileSync(...)` during a transitive module's init) throws a `TypeError` that aborts app mount. A _synchronous_ such throw fires `pageerror`; its _async_ twin (the same `TypeError` via `await`/`.then()`, or a failed dynamic import) is logged on the console channel as `Uncaught (in promise) …` / `Failed to fetch dynamically imported module` — the smoke hard-fails on both. The literal `Module "…" has been externalized` text is, **in a prod build**, a build-time warning (`vite build` / `npm run build`), not a runtime message, so the browser never sees it (under `npm run dev` Vite's stub is instead a `Proxy` that `console.warn`s that string at runtime); and an externalized import that is never _called_ ships a harmless `{}` and is invisible here by design. Every _other_ console error is printed as a diagnostic, not a failure (so a benign font-CDN or React-warning `console.error` doesn't flake CI). Playwright is resolved via `createRequire` based at `clients/web/package.json` — a bare `import("playwright")` would resolve relative to `scripts/`, not the cwd, so it can't be reached that way (it only appears to work when an ancestor `node_modules` carries playwright, and fails in CI, which has none). The npm script's `cd clients/web` exists only so `npx playwright install chromium` finds the local playwright bin (a no-op when already installed). - `smoke:web:app` (`scripts/smoke-web-app.mjs`, #1859) goes one step further again: `smoke:web:browser` stops at first paint and never connects to a server, so the Apps tab, the sandbox controller, and the UI-protocol bridge were unexercised by any smoke. This one boots the same prod `--web` server, spawns the `mcp-app-http.json` composable test server (the `mcp_app_demo` tool + its `mcp_app_demo_widget` UI resource), and drives the whole **connect → open app → widget ready** chain through a single deep-link navigate (`?serverUrl=…&autoConnect=&openApp=…&appArgs=…&autoOpen=`). The assertion is the `data-app-status="ready"` contract from [clients/web/README.md](clients/web/README.md) — the renderer reports `ready` only once the widget has loaded inside the sandbox iframe _and_ fired `notifications/initialized` back through the bridge, so one attribute covers the sandbox proxy being served, the UI resource loading, and the handshake completing. Two mechanics are load-bearing and easy to get wrong: the test server announces readiness on **stderr** (`console.error` in `server-composable.ts`), so both child streams are piped and scanned — watching stdout alone times out with an empty diagnostic; and its bound port is **not** the config's, because `createTestServerHttp` resolves through `findAvailablePort()`, which walks upward when the configured port is taken — so the smoke parses the announced URL rather than assuming `3130`. **Scope note:** this runs against the repo build tree like every other smoke, so it would _not_ have caught #1859 itself (a packaging failure — the file is always present in-repo); `pack:verify` owns that dimension. It does carry a cheap structural pre-check that the proxy page exists at the path `sandbox-controller.ts` resolves, so a move/rename fails fast with a clear cause instead of an opaque render timeout. +- `smoke:web:elicit` (`scripts/smoke-web-elicitation.mjs`, #1854) is the app-rendered **elicitation** counterpart of `smoke:web:app`: same prod `--web` server and the same deep-link connect, but it then calls `app_choose_option` from the Tools tab, waits for `[data-testid="app-elicitation"][data-app-elicitation-status="ready"]`, clicks a choice **inside the sandboxed app** (two `frameLocator` hops — the trusted sandbox proxy, then the untrusted app), and asserts the app's standard `ElicitResult` comes back in the *tool result*, i.e. that it reached the server rather than merely the host. It then repeats against `app-elicitation-native-http.json` — the same tool and app on a server that never advertised the nested MCP Apps `elicitation` capability — and asserts the **native** elicitation dialog takes it and no app modal is rendered. That second half is the more valuable one: the failure this feature can produce is not "the app doesn't render" but "an app renders when it should not have been offered one", which strands every user of a server that never opted in. Set `SMOKE_SCREENSHOT_DIR` to capture PNGs of the three states (used for PR proof); unset, it asserts only. Two mechanics worth knowing: the main-view tabs are a Mantine `SegmentedControl`, so there is no `role="tab"` — the clickable element is the sibling `label[for$="-Tools"]`; and the prompt string also appears in the (hidden) Protocol-tab payload, so the fallback assertion is scoped to the dialog rather than a bare text lookup. + - **The build gate for the browser-externalized-builtin class (#1769)** is the earlier, more complete companion to `smoke:web:browser`. A Vite plugin in `clients/web/vite.config.ts` (logic in `clients/web/server/browser-externalized-builtin-gate.ts`, unit-tested) turns Vite 8's _browser-externalization warning_ (`Module "node:*" has been externalized for browser compatibility`) into a hard `vite build` error, so a Node built-in in the browser graph now **fails `npm run build` / `validate`** instead of shipping a `{}` stub. This catches **both** the _called-at-init_ case (which `smoke:web:browser` also catches, but later/at runtime) **and** the _imported-but-never-called_ case (the `{}` stub that is invisible to the runtime smoke "by design" — see above). Because rolldown **swallows a throw inside `onLog`** (the one hook where a thrown error doesn't abort — verified against vite@8.0.0), the plugin _records_ the warning in `onLog` and re-throws in `buildEnd`. There is **no stable log `code`**, so the gate keys off the documented message phrasing; `npm run verify:build-gate` (`scripts/verify-build-gate.mjs`, in `npm run ci` and the GitHub workflow) runs a real build with a `node:fs` probe forced into `src/main.tsx` and asserts the build fails via the gate — the only check that catches the message phrasing **drifting** in a future Vite bump and silently disabling the gate. The gate is scoped to `vite build` (`apply: 'build'`) — never `vite dev` or the vitest projects — **and** to the browser (`client`) environment (`applyToEnvironment`), so a future SSR/node environment built from this config isn't failed for a legitimate `node:*` import; the Node runner build (tsup, `build:runner`) is a separate config where built-ins are legitimate. `smoke:web:browser` stays as the runtime backstop for crashes the build can't reason about. - `smoke:cli` (`scripts/smoke-cli.mjs`) drives `mcp-inspector --cli` through the built launcher against the bundled stdio test server via a temp `--catalog`: it asserts `tools/list` returns the server's tools (real connect over stdio), the default writable catalog is seeded empty on first run, a missing read-only `--config` errors without seeding, and `--catalog` + `--config` is rejected. `smoke:tui` (`scripts/smoke-tui.mjs`) launches `mcp-inspector --tui --catalog ` and asserts the Ink app renders its first frame (the "MCP Servers" panel) within a timeout, then SIGTERMs it — a shallow boot/render check, not full interaction. **`smoke:tui` is local-only: it self-skips when `process.env.CI` is set**, because the Ink TUI needs a real TTY (raw mode) that headless CI lacks — so run it (via `npm run smoke`) on your own machine before pushing. Both build `test-servers/build` on demand if it's missing. - Storybook play-function tests (`clients/web` `test:storybook`) run in headless Chromium via `@vitest/browser-playwright` (~10s). They are part of `npm run ci` (which installs Playwright chromium first); kept out of `validate` because they need the browser binary and are slower than the unit suite. diff --git a/README.md b/README.md index 2c420c00a3..a577f039f1 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 three 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, and an **app-rendered elicitation** smoke (`smoke:web:elicit`) that drives one 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..a61ad89e80 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,11 @@ 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, + type AppElicitationSession, +} from "./lib/appElicitationController"; import type { LogEntryData } from "./components/elements/LogEntry/LogEntry"; import { ServerConfigModal, @@ -741,6 +753,7 @@ function App() { sandboxUrl, writable: serverListWritable, version: inspectorVersion, + loading: initialConfigLoading, } = useInitialConfig({ baseUrl: configBaseUrl, authToken: getAuthToken(), @@ -792,6 +805,101 @@ 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; + // The window onto the controller for the CURRENT client. Closing it when the + // client is replaced both rejects that connection's queued requests and + // refuses any it enqueues during its own (asynchronous) teardown — a late + // entry would otherwise be rendered by a factory bound to the replacement + // client, i.e. read and answered through a different server. + const appElicitationSessionRef = useRef(null); + // `setupClientForServer` is synchronous and memoized, so a caller that + // awaited the config would still resume with the `sandboxUrl` captured by the + // render it STARTED in — undefined, on the very load this matters for. The + // ref is written every render, so client construction reads the current value + // whichever entry point (connect, deep link, OAuth callback) reached it. + const sandboxUrlRef = useRef(undefined); + sandboxUrlRef.current = sandboxUrl; + // Whether the sandbox exists is only known once `/api/config` resolves, and + // the answer is baked into the client at construction (it decides whether the + // nested MCP Apps `elicitation` capability is advertised). So a connect waits + // for it rather than guessing: guessing "available" over-claims a capability + // we may not have, and guessing "unavailable" strands the whole session on + // the native form despite having a sandbox. The wait is a local fetch already + // in flight since mount. + const initialConfigSettledRef = useRef<{ + promise: Promise; + resolve: () => void; + }>(null); + initialConfigSettledRef.current ??= (() => { + let resolve!: () => void; + const promise = new Promise((r) => (resolve = r)); + return { promise, resolve }; + })(); + useEffect(() => { + if (!initialConfigLoading) initialConfigSettledRef.current?.resolve(); + }, [initialConfigLoading]); + 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], + ); + /** + * Close the previous client's session and open one for the client being + * constructed. Synchronous, and called at construction, so the swap itself is + * the moment ownership changes hands. + */ + const newAppElicitationSession = useCallback(() => { + appElicitationSessionRef.current?.close( + new Error("Connection replaced before the app answered"), + ); + const session = appElicitationController.openSession(); + appElicitationSessionRef.current = session; + return session; + }, [appElicitationController]); + 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 +2518,16 @@ function App() { // Sampling / elicitation are on by default; keep the parameterized // options off until the UI grows the surface to render them. elicit: { form: true, url: true }, + // Web only, and only when the sandbox renderer is actually available: + // supplying this advertises the nested MCP Apps `elicitation` + // capability, and a client that cannot host an app must not claim it + // (#1854). Callers await `initialConfigSettled` first, so `sandboxUrl` + // here means "confirmed absent" rather than "not known yet" — a + // connection that reaches this with no sandbox behaves like the + // CLI/TUI: native elicitation queue, no claim made to the server. + ...(sandboxUrlRef.current && { + appElicitation: newAppElicitationSession().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 +2629,7 @@ function App() { sessionStorageAdapter, onBeforeOAuthRedirect, clientConfig, + newAppElicitationSession, ], ); @@ -2630,6 +2749,9 @@ function App() { } void (async () => { + // Same reason as the connect path: whether this client may advertise + // app-rendered elicitation is fixed at construction. + await initialConfigSettledRef.current?.promise; try { await webOAuthStorage.load(); } catch (err) { @@ -2730,6 +2852,12 @@ function App() { const onToggleConnection = useCallback( async (id: string) => { + // Whether this client may advertise app-rendered elicitation is decided + // at construction and cannot be revised afterwards, so wait for the fact + // rather than guess it (see `initialConfigSettledRef`). Already resolved + // by the time any human clicks; this only orders a deep-link auto-connect + // that races the same page load. + await initialConfigSettledRef.current?.promise; // Same server, already connected → disconnect. if ( id === activeServerId && @@ -4571,6 +4699,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..4baeae7492 --- /dev/null +++ b/clients/web/src/components/elements/AppElicitation/AppElicitationHost.test.tsx @@ -0,0 +1,447 @@ +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, + sessionId: 0, + 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("gives the focus trap, Escape and the overlay to the top modal only", async () => { + // Every entry stays mounted (each app keeps its own bridge and handshake), + // but only one may own the keyboard — otherwise the traps fight and a + // single Escape can dismiss more than one pending request. + const first = createMockBridge({}); + const second = createMockBridge({}); + const bridges = [first.bridge, second.bridge]; + const factory = vi.fn(() => bridges.shift()) as unknown as BridgeFactory; + const user = userEvent.setup(); + + renderWithMantine( + , + ); + await flushAsync(); + expect(screen.getAllByTestId("app-elicitation")).toHaveLength(2); + + // The covered dialog is inert: out of the a11y tree and out of focus + // order, but still mounted — its app keeps its bridge and handshake. + const [lower, top] = screen.getAllByTestId("app-elicitation"); + expect(lower.hasAttribute("inert")).toBe(true); + expect(top.hasAttribute("inert")).toBe(false); + + await user.keyboard("{Escape}"); + // Exactly one request is dismissed — the topmost, which is the last one. + expect(onFail).toHaveBeenCalledTimes(1); + expect(onFail.mock.calls[0][0]).toBe("req-2"); + expect(onFail.mock.calls[0][1].message).toMatch(/dismissed/); + }); + + 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..21677bf558 --- /dev/null +++ b/clients/web/src/components/elements/AppElicitation/AppElicitationHost.tsx @@ -0,0 +1,300 @@ +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, index) => ( + + ))} + + ); +} + +interface AppElicitationFrameProps { + entry: AppElicitationEntry; + /** + * Whether this modal is the topmost of the open set. + * + * Concurrent elicitations mean several are open at once, and each would + * otherwise install its own focus trap, Escape handler and overlay — which + * fight over the keyboard and let one Escape dismiss more than one pending + * request. Only the top one owns those; the rest stay mounted (their apps + * keep their bridges and their handshakes) but inert to the keyboard. + */ + isTop: boolean; + 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, + isTop, + 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 ( + + {isTop && } + {/* `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 or per app: two concurrent + elicitations can be for the SAME app URI with the SAME message, and + only the request id tells those two dialogs apart — for + `landmark-unique` and for anyone navigating by screen reader. */} + + + + + {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..8a111e8fae --- /dev/null +++ b/clients/web/src/components/elements/AppRenderer/appCapabilities.test.ts @@ -0,0 +1,238 @@ +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, + overrides: Record = {}, +): JSONRPCMessage { + return { + jsonrpc: "2.0", + id: 1, + method: "ui/initialize", + params: { + protocolVersion: "2026-01-26", + appInfo: { name: "test-app", version: "1.0.0" }, + appCapabilities, + ...overrides, + }, + } 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 when the bridge advertises nothing at all", () => { + expect(appAdvertisesElicitation(makeBridge(undefined))).toBe(false); + }); + + 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("ignores frames that are not shaped like ui/initialize at all", () => { + // The frame comes from sandboxed view code, so nothing about its shape is + // guaranteed — a non-object, or an initialize with no params, must not + // throw on the way through to the bridge's own handler. + const bridge = makeBridge({}); + const { transport, inner } = makeTransport(); + observeAppCapabilities(bridge, transport); + transport.onmessage?.("not-a-frame" as unknown as JSONRPCMessage); + transport.onmessage?.(null as unknown as JSONRPCMessage); + transport.onmessage?.({ + jsonrpc: "2.0", + id: 1, + method: "ui/initialize", + } as unknown as JSONRPCMessage); + transport.onmessage?.({ + jsonrpc: "2.0", + id: 2, + method: "ui/initialize", + params: null, + } as unknown as JSONRPCMessage); + expect(appAdvertisesElicitation(bridge)).toBe(false); + expect(inner).toHaveBeenCalledTimes(4); + }); + + it("ignores a malformed initialize the bridge would reject", () => { + // Fail-closed: accepting one would let a view flip `elicitation` on with a + // frame that never negotiated anything, and the host would then forward a + // request the bridge does not consider negotiated. + const bridge = makeBridge({}); + const { transport } = makeTransport(); + observeAppCapabilities(bridge, transport); + + // No protocolVersion. + transport.onmessage?.({ + jsonrpc: "2.0", + id: 1, + method: "ui/initialize", + params: { appInfo: { name: "a" }, appCapabilities: { elicitation: {} } }, + } as unknown as JSONRPCMessage); + // No appInfo. + transport.onmessage?.({ + jsonrpc: "2.0", + id: 2, + method: "ui/initialize", + params: { + protocolVersion: "2026-01-26", + appCapabilities: { elicitation: {} }, + }, + } as unknown as JSONRPCMessage); + // A notification, not the handshake request. + transport.onmessage?.({ + jsonrpc: "2.0", + method: "ui/initialize", + params: { + protocolVersion: "2026-01-26", + appInfo: { name: "a" }, + appCapabilities: { elicitation: {} }, + }, + } as unknown as JSONRPCMessage); + + expect(appAdvertisesElicitation(bridge)).toBe(false); + }); + + it("takes the latest accepted handshake, in both directions", () => { + // Verified against ext-apps 1.7.5: a second `ui/initialize` is accepted — + // the bridge warns about the double-mount and the latest appInfo and + // capabilities REPLACE the previous ones. Freezing this at the first frame + // would leave the gate reporting capabilities the bridge no longer holds. + const bridge = makeBridge({}); + const { transport } = makeTransport(); + observeAppCapabilities(bridge, transport); + + transport.onmessage?.(initializeFrame({ availableDisplayModes: [] })); + expect(appAdvertisesElicitation(bridge)).toBe(false); + transport.onmessage?.(initializeFrame({ elicitation: {} })); + expect(appAdvertisesElicitation(bridge)).toBe(true); + // …and a re-handshake that drops the capability turns it back off. + transport.onmessage?.(initializeFrame({})); + expect(appAdvertisesElicitation(bridge)).toBe(false); + }); + + it("leaves a recorded capability alone when a later frame is malformed", () => { + // A rejected frame is not a route to changing the gate — in either + // direction: it cannot set `elicitation`, and it cannot clear one the + // bridge still holds. + const bridge = makeBridge({}); + const { transport } = makeTransport(); + observeAppCapabilities(bridge, transport); + transport.onmessage?.(initializeFrame({ elicitation: {} })); + transport.onmessage?.({ + jsonrpc: "2.0", + id: 9, + method: "ui/initialize", + params: { appCapabilities: {} }, + } as unknown as JSONRPCMessage); + expect(appAdvertisesElicitation(bridge)).toBe(true); + }); + + it("rejects a frame the bridge's own schema rejects", () => { + // `appInfo: {}` is missing the implementation name/version that + // `McpUiInitializeRequestSchema` requires, so the bridge answers an error + // and stores nothing — this gate must not be the laxer of the two. + const bridge = makeBridge({}); + const { transport } = makeTransport(); + observeAppCapabilities(bridge, transport); + transport.onmessage?.({ + jsonrpc: "2.0", + id: 1, + method: "ui/initialize", + params: { + protocolVersion: "2026-01-26", + appInfo: {}, + appCapabilities: { elicitation: {} }, + }, + } as unknown as JSONRPCMessage); + expect(appAdvertisesElicitation(bridge)).toBe(false); + }); + + it("does not treat a non-object elicitation value as an advertisement", () => { + // The draft declares `elicitation` as an object; `true` is a value it does + // not define, and truthiness would accept it. + const bridge = makeBridge({}); + const { transport } = makeTransport(); + observeAppCapabilities(bridge, transport); + transport.onmessage?.(initializeFrame({ elicitation: true })); + expect(appAdvertisesElicitation(bridge)).toBe(false); + transport.onmessage?.(initializeFrame({ elicitation: [] })); + expect(appAdvertisesElicitation(bridge)).toBe(false); + transport.onmessage?.(initializeFrame({ elicitation: {} })); + expect(appAdvertisesElicitation(bridge)).toBe(true); + }); + + 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..9dbac05769 --- /dev/null +++ b/clients/web/src/components/elements/AppRenderer/appCapabilities.ts @@ -0,0 +1,138 @@ +import { McpUiInitializeRequestSchema } from "@modelcontextprotocol/ext-apps/app-bridge"; +import type { AppBridge } from "@modelcontextprotocol/ext-apps/app-bridge"; + +/** + * 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>(); + +/** + * The only part of a transport this needs: the inbound-message callback. + * + * Structural, and generic over the message type, on purpose. ext-apps' + * `PostMessageTransport` implements the SDK *v1* `Transport` — a different + * nominal type from the v2 client's, though runtime-identical — so naming + * either would force a cast at the call site. Generic rather than + * `(message: unknown)` because a handler typed for a narrower message is not + * assignable to one typed for `unknown` (contravariance), which would put the + * cast back; `rest` is `never[]` for the same reason, accepting any trailing + * parameter list without claiming to know it. The message is only ever read + * through {@link recordAdvertisedCapabilities}, which narrows from `unknown`. + */ +export interface MessageObservable { + onmessage?: (message: TMessage, ...rest: never[]) => void; +} + +/** + * The capabilities a view advertised in an `ui/initialize` the bridge will + * ACCEPT, or `undefined` for any other frame. + * + * Acceptance is decided by the bridge's own `McpUiInitializeRequestSchema` + * rather than a hand-rolled approximation of it — an approximation drifts, and + * anything it waves through that the bridge rejects becomes a second, laxer + * route to setting `elicitation` on a frame that negotiated nothing. The one + * check the schema cannot make is that this is a request at all: a JSON-RPC + * notification carries the same `method`/`params`, so the id is checked here. + * + * The capabilities are then read from the ORIGINAL frame, not the parse output, + * because that schema is exactly what strips `elicitation` (see the WeakMap's + * doc comment) — validating with it and reading through it would defeat the + * purpose of this module. + */ +function acceptedInitializeCapabilities( + message: unknown, +): Record | undefined { + if (typeof message !== "object" || message === null) return undefined; + const frame = message as { id?: unknown; params?: unknown }; + // A notification (no id) is not the handshake request. + if (frame.id === undefined || frame.id === null) return undefined; + if (!McpUiInitializeRequestSchema.safeParse(message).success) { + return undefined; + } + const advertised = (frame.params as { appCapabilities?: unknown }) + .appCapabilities; + return typeof advertised === "object" && advertised !== null + ? (advertised as Record) + : undefined; +} + +/** + * Record `params.appCapabilities` from the view's handshake. + * + * Every *accepted* `ui/initialize` replaces the recorded value, because that is + * what the bridge does — on a second handshake (a view double-mounting under + * React StrictMode, or reconnecting) it warns and takes the latest appInfo and + * capabilities. Freezing this at the first frame would leave the gate reporting + * capabilities the bridge no longer holds, in both directions. + * + * A frame the bridge would *reject* records nothing and leaves the previous + * value alone: it is a route to changing this gate in neither direction. + */ +function recordAdvertisedCapabilities( + bridge: AppBridge, + message: unknown, +): void { + const advertised = acceptedInitializeCapabilities(message); + if (advertised) rawAppCapabilities.set(bridge, advertised); +} + +/** + * 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: MessageObservable, +): void { + const inner = transport.onmessage?.bind(transport); + transport.onmessage = (message: TMessage, ...rest: never[]) => { + recordAdvertisedCapabilities(bridge, message); + inner?.(message, ...rest); + }; +} + +/** + * 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; + return ( + isCapabilityObject(parsed?.elicitation) || + isCapabilityObject(rawAppCapabilities.get(bridge)?.elicitation) + ); +} + +/** + * A declared capability is an OBJECT (`{}` today, room for sub-options later), + * which is what ext-apps#733 specifies. Truthiness is not the same test: an + * `elicitation: true` is a value the draft does not define, and treating it as + * an advertisement would accept something the bridge's own schema will reject + * the moment it carries the key. + */ +function isCapabilityObject(value: unknown): boolean { + return typeof value === "object" && value !== null && !Array.isArray(value); +} 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..7f32c26baa 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); @@ -156,6 +158,33 @@ describe("createAppBridgeFactory", () => { } }); + it("does not advertise hostCapabilities.elicitation by default (#1854)", async () => { + // An App-tool frame is never handed an elicitation, so claiming the host + // capability there would tell those apps something untrue about the host. + const factory = createAppBridgeFactory({ + getClient: () => fakeClient, + readResource: vi.fn(), + }); + await factory(makeIframe(), { kind: "tool", tool }); + expect(bridgeInstances[0].ctorArgs[2]).not.toHaveProperty("elicitation"); + }); + + it("advertises hostCapabilities.elicitation when opted in (#1854)", async () => { + // The value reaches the AppBridge CONSTRUCTOR, which is what the bridge + // echoes in its `ui/initialize` response — an app reads it there to decide + // whether this host will forward an elicitation to it at all. + const factory = createAppBridgeFactory({ + advertiseElicitation: true, + getClient: () => fakeClient, + readResource: vi.fn(), + }); + await factory(makeIframe(), { + kind: "resource", + resourceUri: "ui://demo/pick.html", + }); + expect(bridgeInstances[0].ctorArgs[2]).toMatchObject({ elicitation: {} }); + }); + it("on sandboxready, reads the UI resource, wraps the html with the per-app CSP, and echoes the approved sandbox config", async () => { const readResource = vi.fn().mockResolvedValue( uiResource("

weather

", { @@ -167,7 +196,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 +244,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 +264,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 +285,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 +303,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 +324,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 +346,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 +364,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 +381,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 +405,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 +423,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..1447d35978 100644 --- a/clients/web/src/components/elements/AppRenderer/createAppBridgeFactory.ts +++ b/clients/web/src/components/elements/AppRenderer/createAppBridgeFactory.ts @@ -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); 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({ (); +function defaultSession( + controller: AppElicitationController, +): AppElicitationSession { + let session = sessions.get(controller); + if (!session) { + session = controller.openSession(); + sessions.set(controller, session); + } + return session; +} + +describe("AppElicitationController (#1854)", () => { + 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("closing a session drops the entries that session queued", async () => { + // The host closes the old session when the InspectorClient is replaced: the + // bridge factory resolves its client at call time, so an entry left queued + // could otherwise rebuild against the NEXT connection and answer through a + // different server. + const controller = new AppElicitationController(); + const session = controller.openSession(); + const listener = vi.fn(); + const first = request(controller, "a", "ui://a", undefined, session); + const second = request(controller, "b", "ui://b", undefined, session); + controller.subscribe(listener); + + session.close(new Error("connection replaced")); + + await expect(first).rejects.toThrow(/connection replaced/); + await expect(second).rejects.toThrow(/connection replaced/); + expect(controller.getEntries()).toHaveLength(0); + expect(listener).toHaveBeenCalledTimes(1); + }); + + it("refuses a request a closed session makes during its own teardown", async () => { + // The half a one-shot sweep cannot provide: the replaced client + // disconnects asynchronously and can still enqueue, and that entry would be + // rendered by a factory bound to the REPLACEMENT client. + const controller = new AppElicitationController(); + const session = controller.openSession(); + session.close(new Error("connection replaced")); + await expect( + request(controller, "late", "ui://a", undefined, session), + ).rejects.toThrow(/session is closed/); + expect(controller.getEntries()).toHaveLength(0); + }); + + it("leaves another session's entries alone", async () => { + const controller = new AppElicitationController(); + const oldSession = controller.openSession(); + const newSession = controller.openSession(); + const stale = request(controller, "a", "ui://a", undefined, oldSession); + const live = request(controller, "b", "ui://b", undefined, newSession); + + oldSession.close(new Error("connection replaced")); + + await expect(stale).rejects.toThrow(/connection replaced/); + expect(controller.getEntries().map((e) => e.requestId)).toEqual(["b"]); + controller.settle("b", { action: "cancel" }); + await expect(live).resolves.toEqual({ action: "cancel" }); + }); + + it("closing a session with nothing queued notifies nobody", () => { + const controller = new AppElicitationController(); + const session = controller.openSession(); + const listener = vi.fn(); + controller.subscribe(listener); + session.close(new Error("nothing to drop")); + expect(listener).not.toHaveBeenCalled(); + }); + + 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..8ba26b4355 --- /dev/null +++ b/clients/web/src/lib/appElicitationController.ts @@ -0,0 +1,157 @@ +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 { + /** The session (one InspectorClient) this request belongs to. */ + sessionId: number; + /** 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; +} + +/** + * One client's window onto the controller. + * + * Requests are bound to the connection that made them. A closed session + * rejects immediately, which is what a one-shot sweep cannot do: an + * `InspectorClient` being replaced disconnects asynchronously and can still + * enqueue during its own teardown, and that late entry would otherwise be + * rendered by a factory bound to the *replacement* client — reading its + * resource, and answering, through a different server. + */ +export interface AppElicitationSession { + /** The renderer handed to this client's `InspectorClient`. */ + render: AppElicitationRenderer; + /** Reject everything from this session and refuse anything later. */ + close: (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>(); + private nextSessionId = 0; + /** Sessions still accepting requests. A closed one is removed. */ + private openSessions = new Set(); + + /** 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(); + } + + /** + * Open a window for one `InspectorClient`. Its `render` is what that client + * is constructed with; `close` ends it when the client is replaced. + */ + openSession(): AppElicitationSession { + const sessionId = this.nextSessionId++; + this.openSessions.add(sessionId); + return { + render: (request) => this.render(sessionId, request), + close: (error) => this.closeSession(sessionId, error), + }; + } + + /** + * Queue a request for the UI and resolve 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. + * A request from a closed session is rejected without ever being queued. + */ + private render = (sessionId: number, request: AppElicitationRequest) => + new Promise((resolve, reject) => { + if (!this.openSessions.has(sessionId)) { + reject(new Error("App elicitation session is closed")); + return; + } + const entry: AppElicitationEntry = { + ...request, + sessionId, + 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); + } + + /** + * End a session: reject everything it queued, and refuse anything it queues + * later. The refusal is the half a one-shot sweep cannot provide — see + * {@link AppElicitationSession}. + */ + private closeSession(sessionId: number, error: Error): void { + this.openSessions.delete(sessionId); + const dropped = this.entries.filter((e) => e.sessionId === sessionId); + if (dropped.length === 0) return; + this.entries = this.entries.filter((e) => e.sessionId !== sessionId); + this.emit(); + for (const entry of dropped) entry.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..5f0cd45bd4 --- /dev/null +++ b/clients/web/src/test/core/mcp/appElicitation.test.ts @@ -0,0 +1,286 @@ +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(/invalid elicitation result/); + }); + + it("rejects an unknown action", () => { + expect( + validateAppElicitResult(provider, params, { + action: "maybe", + } as unknown as ElicitResult), + ).toMatch(/invalid elicitation result/); + }); + + it("rejects a decline whose content the standard result forbids", () => { + // Checking `action` alone would wave this through: `ElicitResult` permits + // only primitives and string arrays in `content`, so a nested object + // would reach the server as a result it can legitimately reject. + expect( + validateAppElicitResult(provider, params, { + action: "decline", + content: { x: {} }, + } as unknown as ElicitResult), + ).toMatch(/invalid elicitation result/); + }); + + 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(/invalid elicitation result|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..b8b2a6c8b5 --- /dev/null +++ b/clients/web/src/test/core/mcp/inspectorClient-app-elicitation.test.ts @@ -0,0 +1,661 @@ +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 { + return this.sendRequest(id, "elicitation/create", params, timeoutMs); + } + + /** + * Deliver any server→client request and resolve with the client's reply. + * NOT named `send` — that is the Transport method the client calls. + */ + sendRequest( + id: number, + method: string, + params?: Record, + timeoutMs = 2000, + ): Promise { + const reply = new Promise((resolve) => { + this.waiters.set(id, resolve); + }); + this.deliver({ jsonrpc: "2.0", id, method, ...(params && { 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)); + } + + /** Deliver a server→client notification (no reply expected). */ + notify(method: string, params?: Record): void { + this.deliver({ + jsonrpc: "2.0", + method, + ...(params && { params }), + } as unknown as JSONRPCMessage); + } + + 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, + }; +} + +/** The receiver tasks the client reports over `tasks/list`, by status. */ +async function taskStatuses( + transport: ElicitTransport, + id: number, +): Promise { + const reply = (await transport.sendRequest(id, "tasks/list")) as { + result?: { tasks?: { status?: string }[] }; + }; + return (reply.result?.tasks ?? []).map((t) => t.status ?? ""); +} + +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("task-augmented inbound requests", () => { + /** + * A `params.task` elicitation answers the server immediately with a + * `CreateTaskResult` and settles the TASK when the user answers, so it + * cannot ride `enqueuePendingElicitation`. It is still an + * `elicitation/create`, so the app-rendering contract applies to it too. + */ + async function connectWithTasks( + transport: ElicitTransport, + renderer?: AppElicitationRenderer, + ) { + const client = new InspectorClient( + { type: "stdio", command: "noop", args: [] }, + { + environment: { transport: () => ({ transport }) }, + elicit: { form: true }, + receiverTasks: true, + ...(renderer && { appElicitation: renderer }), + }, + ); + await client.connect(); + return client; + } + + const taskParams = () => appParams({ task: { ttl: 60_000 } }); + + it("renders the app and completes the task with its result", async () => { + const transport = new ElicitTransport(); + const seen: AppElicitationRequest[] = []; + const client = await connectWithTasks(transport, async (request) => { + seen.push(request); + return { action: "accept", content: { choice: "option-a" } }; + }); + + // The immediate response is the task handle, not the answer. + const reply = await transport.elicit(50, taskParams()); + expect(reply).toMatchObject({ + id: 50, + result: { task: { taskId: expect.any(String) } }, + }); + + await vi.waitFor(() => expect(seen).toHaveLength(1)); + expect(seen[0].resourceUri).toBe(APP_URI); + // The app answered, so the native queue never opened and the task + // completed on its own — read back the way a server reads it, over + // `tasks/list`. + expect(client.getPendingElicitations()).toHaveLength(0); + await vi.waitFor(async () => + expect(await taskStatuses(transport, 60)).toContain("completed"), + ); + await client.disconnect(); + }); + + it("tears the app down on tasks/cancel and keeps the task cancelled", async () => { + // `tasks/cancel` is the server changing its mind. Without an abort the + // modal stayed live, and a later answer overwrote `cancelled` with + // `completed` — a task the server was told it had cancelled. + const transport = new ElicitTransport(); + let aborted = false; + let settle: ((r: ElicitResult) => void) | undefined; + const client = await connectWithTasks( + transport, + (request) => + new Promise((resolve) => { + settle = resolve; + request.signal.addEventListener("abort", () => (aborted = true)); + }), + ); + + const created = (await transport.elicit(52, taskParams())) as { + result?: { task?: { taskId?: string } }; + }; + const taskId = created.result?.task?.taskId; + expect(taskId).toBeDefined(); + await vi.waitFor(() => expect(settle).toBeDefined()); + + await transport.sendRequest(53, "tasks/cancel", { taskId }); + expect(aborted).toBe(true); + expect(await taskStatuses(transport, 54)).toContain("cancelled"); + + // An answer that lands after the cancel must not resurrect the task. + settle?.({ action: "accept", content: { choice: "too-late" } }); + await vi.waitFor(async () => + expect(await taskStatuses(transport, 55)).toEqual(["cancelled"]), + ); + await client.disconnect(); + }); + + it("falls back to the native queue when the app cannot answer", async () => { + const transport = new ElicitTransport(); + const client = await connectWithTasks(transport, async () => { + throw new Error("sandbox unavailable"); + }); + await transport.elicit(51, taskParams()); + await vi.waitFor(() => + expect(client.getPendingElicitations()).toHaveLength(1), + ); + await client + .getPendingElicitations()[0] + .respond({ action: "accept", content: { choice: "native" } }); + await vi.waitFor(async () => + expect(await taskStatuses(transport, 61)).toContain("completed"), + ); + await client.disconnect(); + }); + }); + + describe("request-scoped ids", () => { + it("mints ids that are unique across client instances, not just within one", async () => { + // A host may hold ONE renderer across replacement clients (the web client + // rebuilds its InspectorClient on a settings change). A per-client + // counter alone would give each new client's first request the same id, + // and settling it would resolve the wrong server's request. + const ids: string[] = []; + const renderer: AppElicitationRenderer = async (request) => { + ids.push(request.requestId); + return { action: "cancel" }; + }; + const first = new ElicitTransport(); + const clientA = await connectClient({ + transport: first, + appElicitation: renderer, + }); + await first.elicit(40, appParams()); + await clientA.disconnect(); + + const second = new ElicitTransport(); + const clientB = await connectClient({ + transport: second, + appElicitation: renderer, + }); + await second.elicit(41, appParams()); + await clientB.disconnect(); + + expect(ids).toHaveLength(2); + expect(ids[0]).not.toBe(ids[1]); + }); + }); + + 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); + }); + + it("tears the app down when the server cancels the request", async () => { + // A direct server→client `elicitation/create` can be cancelled with + // `notifications/cancelled`; the SDK aborts `ctx.mcpReq.signal` for it. + // Without that signal threaded through, the modal and its bridge outlive + // the request and could answer work the server abandoned. + 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(32, appParams()).catch(() => {}); + await vi.waitFor(() => expect(aborted).toBe(false)); + + transport.notify("notifications/cancelled", { + requestId: 32, + reason: "server changed its mind", + }); + + await vi.waitFor(() => expect(aborted).toBe(true)); + // Not reopened natively: the request the user was answering is gone. + expect(client.getPendingElicitations()).toHaveLength(0); + await client.disconnect(); + }); + + it("aborts a pending app elicitation on a mid-session transport close", async () => { + // The `onclose` route reaches teardown only through + // `clearAndAnnouncePendingPeerRequests`, whose emptiness check used to + // look at the native queues alone — so a lone app elicitation survived a + // dropped connection and left its modal on screen. + 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(31, appParams()).catch(() => {}); + await vi.waitFor(() => expect(aborted).toBe(false)); + + transport.onclose?.(); + + await vi.waitFor(() => expect(aborted).toBe(true)); + 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..65e02cc1e8 --- /dev/null +++ b/clients/web/src/test/integration/mcp/appElicitation.test.ts @@ -0,0 +1,296 @@ +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 { eraToVersionNegotiation } from "@inspector/core/mcp/types.js"; +import type { AppElicitationRequest } from "@inspector/core/mcp/appElicitation.js"; +import { + APP_ELICITATION_URI, + createAppElicitationResource, + createAppElicitationTool, + createMrtrAppElicitationTool, + 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; + } + + /** The modern MRTR fixture tool, as the server reports it. */ + async function mrtrTool(connected: InspectorClient) { + const { tools } = await connected.listTools(); + const tool = tools.find((t) => t.name === "mrtr_app_choose_option"); + if (!tool) + throw new Error("mrtr_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); + }); + + /** + * The modern (2026-07-28) leg, where an elicitation reaches the client as an + * `input_required` result the MRTR driver unpacks — not as a server→client + * request. Stateless per request, hence a separate server. + */ + async function startModernServer(): Promise { + const started = createTestServerHttp({ + serverInfo: createTestServerInfo("app-elicit-mrtr", "1.0.0"), + tools: [createMrtrAppElicitationTool()], + resources: [createAppElicitationResource()], + appElicitation: true, + modern: {}, + }); + await started.start(); + servers.push(started); + return started; + } + + async function connectModern( + url: string, + renderer?: (request: AppElicitationRequest) => Promise, + ): Promise { + const connected = new InspectorClient( + { type: "streamable-http", url }, + { + environment: { transport: createTransportNode }, + elicit: { form: true }, + versionNegotiation: eraToVersionNegotiation("modern"), + ...(renderer && { appElicitation: renderer }), + }, + ); + await connected.connect(); + client = connected; + return connected; + } + + describe("modern MRTR input_required", () => { + it("routes an embedded elicitation to the app and retries with its result", async () => { + // The other half of the contract: the same routing decision has to hold + // on a leg where the elicitation never arrives as a request at all. + const started = await startModernServer(); + const seen: AppElicitationRequest[] = []; + const connected = await connectModern(started.url, async (request) => { + seen.push(request); + return { action: "accept", content: { choice: "option-b" } }; + }); + + const tool = await mrtrTool(connected); + const result = await connected.callTool(tool, { + prompt: "Choose option A or B.", + }); + + expect(seen).toHaveLength(1); + expect(seen[0].resourceUri).toBe(APP_ELICITATION_URI); + // The retry carried the app's answer as `inputResponses`, and the server + // echoed it — so the app's result completed the ORIGINAL tool call. + expect(JSON.stringify(result.result?.content)).toContain("option-b"); + expect(connected.getPendingElicitations()).toHaveLength(0); + }); + + it("aborts the app when the tool call it belongs to is cancelled", async () => { + // MRTR passes the call's signal down; without it a cancelled tool call + // would leave the app's modal open with nothing left to answer. + const started = await startModernServer(); + let rendering = false; + let aborted = false; + const connected = await connectModern( + started.url, + (request) => + new Promise((_resolve, reject) => { + rendering = true; + request.signal.addEventListener("abort", () => { + aborted = true; + reject(new Error("aborted")); + }); + }), + ); + + const tool = await mrtrTool(connected); + const call = connected.callTool(tool, {}).catch((err: unknown) => err); + // Cancel only once the app is actually up: cancelling earlier is a + // different case (the signal is already aborted and no app is mounted). + await vi.waitFor(() => expect(rendering).toBe(true)); + expect(connected.cancelToolCall()).toBe(true); + await vi.waitFor(() => expect(aborted).toBe(true)); + await call; + }); + + it("falls back to the native UI when the modern server did not advertise it", async () => { + const started = createTestServerHttp({ + serverInfo: createTestServerInfo("app-elicit-mrtr-native", "1.0.0"), + tools: [createMrtrAppElicitationTool()], + resources: [createAppElicitationResource()], + modern: {}, + }); + await started.start(); + servers.push(started); + + let rendererCalls = 0; + const connected = await connectModern(started.url, async () => { + rendererCalls++; + return { action: "cancel" }; + }); + const tool = await mrtrTool(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" } }); + const result = await call; + expect(JSON.stringify(result.result?.content)).toContain("option-a"); + 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..1ea84651a3 --- /dev/null +++ b/core/mcp/appElicitation.ts @@ -0,0 +1,241 @@ +import type { + ClientCapabilities, + ElicitRequest, + ElicitResult, + JsonSchemaType, + jsonSchemaValidator, + ServerCapabilities, +} from "@modelcontextprotocol/client"; +import { ElicitResultSchema } from "@modelcontextprotocol/core"; +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 result contract a completed elicitation must satisfy — the standard MCP + * one, not a hand-rolled subset. Checking `action` by hand would accept, for + * instance, `{ action: "decline", content: { x: {} } }`, whose content the + * schema forbids, and hand the server a result it can reject. + */ +const ELICIT_RESULT_ERROR = "App returned an invalid elicitation result"; + +/** + * 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 { + const parsed = ElicitResultSchema.safeParse(result); + if (!parsed.success) { + return `${ELICIT_RESULT_ERROR}: ${parsed.error.issues[0]?.message ?? "does not match ElicitResult"}`; + } + // decline / cancel carry no content — they are complete as they stand. + if (parsed.data.action !== "accept") return undefined; + const content = parsed.data.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..fb0ad80eee 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, @@ -232,6 +239,14 @@ interface ReceiverTaskRecord { resolvePayload: (payload: ClientResult) => void; rejectPayload: (reason?: unknown) => void; cleanupTimeoutId?: ReturnType; + /** + * Aborted when the task reaches a terminal state some way other than the + * user answering — a `tasks/cancel`, or session teardown. Whatever is + * collecting the answer (the native pending-request entry, or an app-rendered + * elicitation and its bridge) is torn down from this, so a cancelled task + * cannot leave a modal on screen waiting for an answer nothing will read. + */ + abort: AbortController; } /** @@ -562,6 +577,38 @@ 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; + /** + * Per-instance prefix for {@link appElicitationSeq}. + * + * A counter alone is not enough: a host may hold ONE renderer across + * replacement clients (the web client rebuilds its `InspectorClient` on a + * settings change), and every fresh instance would otherwise mint + * `app-elicitation-1` for its first request. Settling that id would then + * resolve — or discard — whichever connection's request the host happened to + * be keying. + */ + private readonly appElicitationPrefix = `app-elicitation-${globalThis.crypto?.randomUUID?.() ?? Math.random().toString(36).slice(2)}`; + /** + * 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 +663,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 +833,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 = { @@ -1174,6 +1230,7 @@ export class InspectorClient extends InspectorClientEventTarget { payloadPromise, resolvePayload, rejectPayload, + abort: new AbortController(), }; record.cleanupTimeoutId = setTimeout(() => { record.cleanupTimeoutId = undefined; @@ -1250,6 +1307,9 @@ export class InspectorClient extends InspectorClientEventTarget { }; record.task = updatedTask; record.rejectPayload(new Error("Task cancelled")); + // Stop collecting an answer nobody will read: drops the native pending + // entry and tears down an app-rendered elicitation's renderer. + record.abort.abort(); if (record.cleanupTimeoutId != null) { clearTimeout(record.cleanupTimeoutId); record.cleanupTimeoutId = undefined; @@ -1379,7 +1439,13 @@ export class InspectorClient extends InspectorClientEventTarget { // an elicit option that enables no mode advertises nothing, and registering // regardless throws before the handshake. if (this.elicitationCapabilityAdvertised && this.client) { - const elicitHandler = (request: ElicitRequest): Promise => { + const elicitHandler = ( + request: ElicitRequest, + // Structural, and only the one field this needs: the SDK's + // `ClientContext` carries much more, and naming it here would tie the + // handler to a type the bypass helper below does not thread through. + ctx?: { mcpReq?: { signal?: AbortSignal } }, + ): Promise => { const paramsTask = (request.params as { task?: { ttl?: number } }) ?.task; if (this.tasksCapabilityAdvertised && paramsTask != null) { @@ -1388,35 +1454,74 @@ export class InspectorClient extends InspectorClientEventTarget { initialStatus: "input_required", statusMessage: "Awaiting user input", }); + // Settling the receiver task, shared by both answer routes below so + // an app-rendered answer completes the task exactly as a native one + // does. + const completeTask = (result: ElicitResult) => { + // A cancelled (or otherwise terminal) task must not be re-settled: + // an answer that arrives after `tasks/cancel` would otherwise + // overwrite `cancelled` with `completed`. + if (InspectorClient.isTerminalTaskStatus(record.task.status)) + return; + record.resolvePayload(result); + const updated: Task = { + ...record.task, + status: "completed", + lastUpdatedAt: new Date().toISOString(), + }; + record.task = updated; + this.upsertReceiverTask(updated); + }; + const failTask = (error: Error) => { + if (InspectorClient.isTerminalTaskStatus(record.task.status)) + return; + record.rejectPayload(error); + const updated: Task = { + ...record.task, + status: "failed", + lastUpdatedAt: new Date().toISOString(), + statusMessage: error.message, + }; + record.task = updated; + this.upsertReceiverTask(updated); + }; void (async () => { + // A task-augmented request is still an `elicitation/create`, so the + // app-rendering contract applies to it too (#1854). It cannot go + // through `enqueuePendingElicitation` — the response frame has + // already been sent as a `CreateTaskResult` and the answer settles + // the TASK rather than the request — so the same attempt is made + // here, falling back to the native queue exactly as that funnel + // does. An abort (disconnect) fails the task rather than reopening + // it natively. + let appResult: ElicitResult | null; + try { + appResult = await this.tryAppElicitation( + request, + record.abort.signal, + ); + } catch (error) { + failTask( + error instanceof Error ? error : new Error(String(error)), + ); + return; + } + if (appResult) { + completeTask(appResult); + return; + } const elicitationRequest = new ElicitationCreateMessage( request, - (result) => { - record.resolvePayload(result); - const now = new Date().toISOString(); - const updated: Task = { - ...record.task, - status: "completed", - lastUpdatedAt: now, - }; - record.task = updated; - this.upsertReceiverTask(updated); - }, + completeTask, (id) => this.removePendingElicitation(id), - (error) => { - record.rejectPayload(error); - const now = new Date().toISOString(); - const updated: Task = { - ...record.task, - status: "failed", - lastUpdatedAt: now, - statusMessage: error.message, - }; - record.task = updated; - this.upsertReceiverTask(updated); - }, + failTask, ); this.addPendingElicitation(elicitationRequest); + // A `tasks/cancel` (or teardown) drops the queued entry, so the + // modal does not outlive the task it belongs to. + this.wirePendingAbort(record.abort.signal, () => + this.removePendingElicitation(elicitationRequest.id), + ); })(); // Task-augmented (2025-11-25) response — see the sampling handler // above. Reply with a `CreateTaskResult` (`{ task }`), routed around @@ -1429,7 +1534,19 @@ export class InspectorClient extends InspectorClientEventTarget { const taskResult: CreateTaskResult = { task: record.task }; return Promise.resolve(taskResult as unknown as ElicitResult); } - return this.enqueuePendingElicitation(request, "server-request"); + // `ctx.mcpReq.signal` aborts when the server cancels this request + // (`notifications/cancelled`). Threading it through means both answer + // surfaces — the native queue entry and an app-rendered elicitation's + // renderer — are torn down with the request, instead of a modal + // outliving work the server abandoned. The task-augmented branch above + // deliberately does NOT use it: that request is answered immediately + // with a `CreateTaskResult`, so its lifetime is the task's, which + // carries its own abort (see `ReceiverTaskRecord.abort`). + return this.enqueuePendingElicitation( + request, + "server-request", + ctx?.mcpReq?.signal, + ); }; this.client.setRequestHandler("elicitation/create", elicitHandler); // Registration, like the `setRequestHandler` above it — and the whole @@ -1548,6 +1665,9 @@ export class InspectorClient extends InspectorClientEventTarget { if (record.cleanupTimeoutId != null) { clearTimeout(record.cleanupTimeoutId); } + // Same reason as `cancelReceiverTask`: the session that owns whatever is + // collecting the answer is ending. + record.abort.abort(); } this.receiverTaskRecords.clear(); } @@ -1688,6 +1808,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(); } /** @@ -1711,7 +1838,12 @@ export class InspectorClient extends InspectorClientEventTarget { private clearAndAnnouncePendingPeerRequests(): void { if ( this.pendingSamples.length === 0 && - this.pendingElicitations.length === 0 + this.pendingElicitations.length === 0 && + // App-rendered elicitations (#1854) are not in either array — they live in + // the host's renderer — so an emptiness check that ignores them lets a + // mid-session close (the `onclose` route, which reaches teardown only + // through here) leave a modal open for a connection that is gone. + this.activeAppElicitations.size === 0 ) { return; } @@ -2820,11 +2952,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 +2980,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. + // Already cancelled before we got here (the caller aborted while an earlier + // MRTR round was in flight): don't mount an app nobody is waiting on. + if (signal?.aborted) throw createPendingAbortError(); + const controller = new AbortController(); + const forwardAbort = () => controller.abort(signal?.reason); + signal?.addEventListener("abort", forwardAbort, { once: true }); + this.activeAppElicitations.add(controller); + try { + const result = await renderer({ + requestId: `${this.appElicitationPrefix}-${++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..d8169f6c86 --- /dev/null +++ b/scripts/smoke-web-elicitation.mjs @@ -0,0 +1,344 @@ +#!/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; +// The async half of the uncaught-crash class. Kept identical to +// smoke-web-browser.mjs / smoke-web-app.mjs. +const FATAL_CONSOLE = /^Uncaught\b|Failed to fetch dynamically imported module/; + +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