test(apps): cover the modern MRTR leg of app-rendered elicitations
Both review findings from round two. The routing change sits in the funnel BOTH entry points share, but only the legacy inbound handler was driven by a test — the modern leg, where an elicitation arrives as an `input_required` result the MRTR driver unpacks and retries, was covered by construction alone. Adds `mrtr_app_choose_option` (a modern MRTR tool whose *embedded* elicitation carries `_meta.ui.resourceUri`) and drives it live over a real transport: the app answers, the retry carries that answer through `inputResponses`, and the server echoes it back as the tool's result. A second case cancels the tool call while the app is up and asserts the request-scoped signal aborts; a third runs the same tool against a modern server that never advertised the capability and asserts the native queue takes it. The cancellation case turned up a real gap: `tryAppElicitation` mounted an app for an already-aborted signal (the caller cancelled during an earlier MRTR round), leaving a modal nobody was waiting on and a promise that could never settle. It now throws the abort straight through. Also fixes the smoke count in the root README, which still said "two" headless-Chromium smokes and then introduced a third. Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
cliffhall
parent
7c77c2b15f
commit
44010c313b
5 changed files with 196 additions and 5 deletions
|
|
@ -363,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. A third (`smoke:web:elicit`) drives an **app-rendered elicitation** end to end — call the tool, answer inside the sandboxed app, see the app's `ElicitResult` reach the server — and then the same tool against a server that never advertised the capability, which must fall back to the native elicitation form. |
|
||||
| `npm run 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. |
|
||||
|
|
|
|||
|
|
@ -2,11 +2,13 @@ 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,
|
||||
|
|
@ -72,6 +74,15 @@ describe("app-rendered elicitation, live server (#1854)", () => {
|
|||
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<ElicitResult>,
|
||||
|
|
@ -150,6 +161,123 @@ describe("app-rendered elicitation, live server (#1854)", () => {
|
|||
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<TestServerHttp> {
|
||||
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<ElicitResult>,
|
||||
): Promise<InspectorClient> {
|
||||
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<ElicitResult>((_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.
|
||||
|
|
|
|||
|
|
@ -2953,12 +2953,12 @@ export class InspectorClient extends InspectorClientEventTarget {
|
|||
// 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);
|
||||
if (signal) {
|
||||
if (signal.aborted) forwardAbort();
|
||||
else signal.addEventListener("abort", forwardAbort, { once: true });
|
||||
}
|
||||
signal?.addEventListener("abort", forwardAbort, { once: true });
|
||||
this.activeAppElicitations.add(controller);
|
||||
try {
|
||||
const result = await renderer({
|
||||
|
|
|
|||
|
|
@ -57,6 +57,7 @@ import {
|
|||
createMcpAppDemoResource,
|
||||
createAppElicitationTool,
|
||||
createAppElicitationResource,
|
||||
createMrtrAppElicitationTool,
|
||||
createArchitectureResource,
|
||||
createTestCwdResource,
|
||||
createTestEnvResource,
|
||||
|
|
@ -227,6 +228,8 @@ function resolveToolPreset(
|
|||
return createMcpAppDemoTool();
|
||||
case "app_choose_option":
|
||||
return createAppElicitationTool();
|
||||
case "mrtr_app_choose_option":
|
||||
return createMrtrAppElicitationTool();
|
||||
default:
|
||||
throw new Error(`Unknown tool preset: ${name}`);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -641,6 +641,66 @@ export function createAppElicitationTool(): ToolDefinition {
|
|||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* The modern (2026-07-28) counterpart of {@link createAppElicitationTool}: an
|
||||
* MRTR tool whose EMBEDDED elicitation carries `_meta.ui.resourceUri` (#1854).
|
||||
*
|
||||
* The two paths reach the Inspector completely differently — a server→client
|
||||
* `elicitation/create` request on the legacy leg, an `input_required` result
|
||||
* the client unpacks and retries on the modern one — and the routing decision
|
||||
* has to be identical on both. It is, because both funnel through the same
|
||||
* `enqueuePendingElicitation`; this fixture is what lets a test prove that
|
||||
* rather than assert it.
|
||||
*
|
||||
* Completes on the retry by echoing the `ElicitResult` the app produced, so a
|
||||
* caller can see the app's answer round-trip through `inputResponses`.
|
||||
*/
|
||||
export function createMrtrAppElicitationTool(): ToolDefinition {
|
||||
return {
|
||||
name: "mrtr_app_choose_option",
|
||||
description:
|
||||
"Modern MRTR tool whose embedded elicitation offers an MCP App to render the form",
|
||||
inputSchema: {
|
||||
prompt: z.string().optional().describe("Message shown above the choice"),
|
||||
},
|
||||
handler: async (
|
||||
params: Record<string, unknown>,
|
||||
_context?: TestServerContext,
|
||||
extra?: HandlerExtra,
|
||||
) => {
|
||||
const responses = extra?.inputResponses;
|
||||
if (!responses || responses.choice === undefined) {
|
||||
return inputRequired({
|
||||
inputRequests: {
|
||||
choice: inputRequired.elicit({
|
||||
message:
|
||||
typeof params.prompt === "string"
|
||||
? params.prompt
|
||||
: "Choose option A or B.",
|
||||
requestedSchema: {
|
||||
type: "object",
|
||||
properties: {
|
||||
choice: {
|
||||
type: "string",
|
||||
enum: ["option-a", "option-b"],
|
||||
title: "Choice",
|
||||
},
|
||||
},
|
||||
required: ["choice"],
|
||||
},
|
||||
_meta: { ui: { resourceUri: APP_ELICITATION_URI } },
|
||||
}),
|
||||
},
|
||||
requestState: `mrtr-app:${++mrtrMintCount}`,
|
||||
});
|
||||
}
|
||||
return toToolResult(
|
||||
`Elicitation response: ${JSON.stringify(responses.choice)}`,
|
||||
);
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Create an "mrtr_confirm" tool exercising the modern (2026-07-28) multi
|
||||
* round-trip request (MRTR) flow. On the first call it returns an
|
||||
|
|
|
|||
Loading…
Reference in a new issue