From e2a88d49dde99f2f66774ad923fa378f3ae75e3c Mon Sep 17 00:00:00 2001 From: cliffhall Date: Sun, 23 Aug 2026 21:50:36 -0400 Subject: [PATCH] fix(web): restore the read result envelope as a metadata carrier MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `McpUiReadResourceResult` — the return type of a registerAppResource read callback — types `_meta.ui` at the result level, so the envelope is a supported carrier after all and dropping it lost the hints for servers using that shape. Consult all three, most-specific first: content item, result envelope, listing entry; first found wins outright. Also make the App wiring test wait for BOTH factories by their advertiseElicitation flag. Waiting on a count passed as soon as either was built, so stripping the wiring from one call site left the other's deps in the array and the assertion loop still passed. Signed-off-by: cliffhall --- clients/web/src/App.test.tsx | 21 ++++-- .../createAppBridgeFactory.test.ts | 73 ++++++++++++++++++- .../AppRenderer/createAppBridgeFactory.ts | 20 +++-- 3 files changed, 99 insertions(+), 15 deletions(-) diff --git a/clients/web/src/App.test.tsx b/clients/web/src/App.test.tsx index 923a5e1f..86a505f2 100644 --- a/clients/web/src/App.test.tsx +++ b/clients/web/src/App.test.tsx @@ -3183,15 +3183,26 @@ describe("App MCP App listed-resource metadata wiring (#2055)", () => { ); renderWithMantine(); - await waitFor(() => expect(appBridgeFactoryDeps.length).toBeGreaterThan(0)); - // Every factory App builds gets the wiring, not just the Apps-tab one. - for (const deps of appBridgeFactoryDeps) { - expect(deps.getListedResourceMeta?.("ui://weather/app.html")).toEqual( + // App builds two factories, differing only in `advertiseElicitation`. Wait + // for BOTH by that flag rather than for a count: waiting on "at least one" + // would still pass with the wiring stripped from either call site, since + // the other's deps sit in the same array. + const appsFactory = () => + appBridgeFactoryDeps.find((d) => !d.advertiseElicitation); + const elicitationFactory = () => + appBridgeFactoryDeps.find((d) => d.advertiseElicitation === true); + await waitFor(() => { + expect(appsFactory()).toBeDefined(); + expect(elicitationFactory()).toBeDefined(); + }); + + for (const deps of [appsFactory(), elicitationFactory()]) { + expect(deps?.getListedResourceMeta?.("ui://weather/app.html")).toEqual( listedMeta, ); expect( - deps.getListedResourceMeta?.("ui://other/app.html"), + deps?.getListedResourceMeta?.("ui://other/app.html"), ).toBeUndefined(); } }); diff --git a/clients/web/src/components/elements/AppRenderer/createAppBridgeFactory.test.ts b/clients/web/src/components/elements/AppRenderer/createAppBridgeFactory.test.ts index 6b16d680..35eeb1cb 100644 --- a/clients/web/src/components/elements/AppRenderer/createAppBridgeFactory.test.ts +++ b/clients/web/src/components/elements/AppRenderer/createAppBridgeFactory.test.ts @@ -86,20 +86,24 @@ const fakeClient = { name: "sdk-client" } as unknown as Client; /** * A UI resource read result. `meta` is the MCP Apps sandbox metadata, which the * spec nests under the `_meta` bag's `ui` key — the helper wraps it so every - * test drives the real wire shape. `opts.flat` writes it to `_meta` unnested - * instead (the non-conforming shape the host must ignore). + * test drives the real wire shape. `opts.at` places the bag on the result + * envelope (`McpUiReadResourceResult`) instead of the content item, and + * `opts.flat` writes it unnested (the non-conforming shape the host ignores). */ function uiResource( text: string | undefined, meta?: Record, - opts: { flat?: boolean } = {}, + opts: { at?: "content" | "result"; flat?: boolean } = {}, ): ReadResourceResult { + const bag = meta ? { _meta: opts.flat ? meta : { ui: meta } } : {}; + const onResult = opts.at === "result"; return { + ...(onResult ? bag : {}), contents: [ { uri: "ui://weather/app.html", ...(text === undefined ? {} : { text }), - ...(meta ? { _meta: opts.flat ? meta : { ui: meta } } : {}), + ...(onResult ? {} : bag), }, ], } as ReadResourceResult; @@ -242,6 +246,67 @@ describe("createAppBridgeFactory", () => { }); }); + it("falls back to the read result's _meta.ui when the content item carries none", async () => { + // `McpUiReadResourceResult` — what a registerAppResource callback returns — + // types `_meta.ui` at the envelope level, so a server stamping it there + // must have it honored. + const readResource = vi.fn().mockResolvedValue( + uiResource( + "

x

", + { + permissions: { microphone: {} }, + csp: { connectDomains: ["https://envelope.example.com"] }, + }, + { at: "result" }, + ), + ); + const factory = createAppBridgeFactory({ + getClient: () => fakeClient, + readResource, + }); + await factory(makeIframe(), { kind: "tool", tool }); + const bridge = bridgeInstances[0]; + bridge.emit("sandboxready"); + await flush(); + + const call = bridge.sendSandboxResourceReady.mock.calls[0][0] as { + html: string; + permissions: unknown; + }; + expect(call.permissions).toEqual({ microphone: {} }); + expect(call.html).toContain("connect-src https://envelope.example.com"); + }); + + it("prefers the read result's _meta.ui over the listing entry's", async () => { + // Precedence runs most-specific first: content item, then the result + // envelope, then the listing default. + const readResource = vi + .fn() + .mockResolvedValue( + uiResource( + "

x

", + { csp: { connectDomains: ["https://envelope.example.com"] } }, + { at: "result" }, + ), + ); + const factory = createAppBridgeFactory({ + getClient: () => fakeClient, + readResource, + getListedResourceMeta: () => + listedMeta({ csp: { connectDomains: ["https://listed.example.com"] } }), + }); + await factory(makeIframe(), { kind: "tool", tool }); + const bridge = bridgeInstances[0]; + bridge.emit("sandboxready"); + await flush(); + + const call = bridge.sendSandboxResourceReady.mock.calls[0][0] as { + html: string; + }; + expect(call.html).toContain("connect-src https://envelope.example.com"); + expect(call.html).not.toContain("listed.example.com"); + }); + it("falls back to the resources/list entry's _meta.ui when the content block carries none", async () => { // ext-apps documents the listing-level `_meta.ui` as the static default for // a UI resource, so an app declaring its CSP only there must still have it diff --git a/clients/web/src/components/elements/AppRenderer/createAppBridgeFactory.ts b/clients/web/src/components/elements/AppRenderer/createAppBridgeFactory.ts index 2480458d..e088fee9 100644 --- a/clients/web/src/components/elements/AppRenderer/createAppBridgeFactory.ts +++ b/clients/web/src/components/elements/AppRenderer/createAppBridgeFactory.ts @@ -131,11 +131,18 @@ function uiMeta(meta: object | undefined): McpUiResourceMeta | undefined { /** * First text content block of a UI resource, plus its `_meta.ui` sandbox hints. * - * `listedMeta` is the `_meta` of the matching `resources/list` entry, which - * ext-apps documents as the static default a host can review at connection - * time; a `resources/read` content item carrying its own `_meta.ui` takes - * precedence over it (`McpUiAppResourceConfig`). Nothing else is consulted — - * in particular the read RESULT's own `_meta` is not a documented carrier. + * ext-apps exposes three carriers, and they are consulted most-specific first: + * + * 1. the **read content item**'s own `_meta.ui`; + * 2. the **read result envelope**'s — `McpUiReadResourceResult` (what a + * `registerAppResource` callback returns) types `_meta.ui` at that level; + * 3. the matching **`resources/list` entry**'s, which `McpUiAppResourceConfig` + * documents as the static default a host reviews at connection time, and + * which a read content item explicitly takes precedence over. + * + * Whichever is found first wins outright — the levels are not merged, since a + * server that restates only `csp` at the more specific level means that to be + * the whole grant, not a patch over the broader one. */ function extractHtmlAndMeta( result: ReadResourceResult, @@ -149,7 +156,8 @@ function extractHtmlAndMeta( if (typeof text === "string") { return { html: text, - meta: uiMeta(content._meta) ?? uiMeta(listedMeta), + meta: + uiMeta(content._meta) ?? uiMeta(result._meta) ?? uiMeta(listedMeta), }; } }