diff --git a/clients/web/src/App.tsx b/clients/web/src/App.tsx index 14f7647f..8ad2dfd4 100644 --- a/clients/web/src/App.tsx +++ b/clients/web/src/App.tsx @@ -2483,11 +2483,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: hands form elicitations that name a `ui://` resource to the - // MCP App the server chose, and advertises the nested MCP Apps - // `elicitation` capability that makes a server willing to send one - // (#1854). The native elicitation queue stays the fallback. - appElicitation: appElicitationController.render, + // 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). `sandboxUrl` is undefined while `/api/config` is in flight + // and when the sandbox controller could not start, so this connection + // simply behaves like the CLI/TUI: the native elicitation queue, with + // no claim made to the server. + ...(sandboxUrl && { + appElicitation: appElicitationController.render, + }), // Always advertise the roots capability (even with no configured // roots) so the server can issue roots/list and receive // roots/list_changed; the configured roots are the answer to @@ -2590,6 +2595,7 @@ function App() { onBeforeOAuthRedirect, clientConfig, appElicitationController.render, + sandboxUrl, ], ); diff --git a/clients/web/src/components/elements/AppElicitation/AppElicitationHost.test.tsx b/clients/web/src/components/elements/AppElicitation/AppElicitationHost.test.tsx index 3f17a19c..d11b28b5 100644 --- a/clients/web/src/components/elements/AppElicitation/AppElicitationHost.test.tsx +++ b/clients/web/src/components/elements/AppElicitation/AppElicitationHost.test.tsx @@ -193,6 +193,35 @@ describe("AppElicitationHost (#1854)", () => { 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); + + 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( diff --git a/clients/web/src/components/elements/AppElicitation/AppElicitationHost.tsx b/clients/web/src/components/elements/AppElicitation/AppElicitationHost.tsx index 1276a724..b158a088 100644 --- a/clients/web/src/components/elements/AppElicitation/AppElicitationHost.tsx +++ b/clients/web/src/components/elements/AppElicitation/AppElicitationHost.tsx @@ -118,10 +118,13 @@ export function AppElicitationHost({ return ( <> - {entries.map((entry) => ( + {entries.map((entry, index) => ( void; @@ -150,6 +163,7 @@ interface AppElicitationFrameProps { */ function AppElicitationFrame({ entry, + isTop, sandboxPath, bridgeFactory, onSettle, @@ -221,8 +235,13 @@ function AppElicitationFrame({ ); 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 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 index 25216b41..c4835bfd 100644 --- a/clients/web/src/test/core/mcp/inspectorClient-app-elicitation.test.ts +++ b/clients/web/src/test/core/mcp/inspectorClient-app-elicitation.test.ts @@ -94,11 +94,24 @@ class ElicitTransport implements Transport { 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: "elicitation/create", params }); + this.deliver({ jsonrpc: "2.0", id, method, ...(params && { params }) }); let timer: ReturnType; return Promise.race([ reply, @@ -125,6 +138,17 @@ function appParams(overrides: Record = {}) { }; } +/** 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; @@ -405,6 +429,78 @@ describe("app-rendered elicitation routing (#1854)", () => { }); }); + 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("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 diff --git a/core/mcp/inspectorClient.ts b/core/mcp/inspectorClient.ts index 4fd47bc3..fd31602a 100644 --- a/core/mcp/inspectorClient.ts +++ b/core/mcp/inspectorClient.ts @@ -1436,33 +1436,57 @@ 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) => { + record.resolvePayload(result); + const updated: Task = { + ...record.task, + status: "completed", + lastUpdatedAt: new Date().toISOString(), + }; + record.task = updated; + this.upsertReceiverTask(updated); + }; + const failTask = (error: Error) => { + 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); + } 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); })();