refactor: drop the ext-apps SDK-v1 shims now that 2.0.0 peers on SDK v2
Every site tagged `ext-apps#702` carried a workaround for ext-apps
peering on the v1 SDK while the Inspector runs v2. With 2.0.0 the
package's `AppBridge` extends the v2 `Protocol`, takes the v2 `Client`
and `Implementation` directly, and registers its list-changed forwarding
on that client with `setNotificationHandler("notifications/…", handler)`,
so:
- core/mcp/apps.ts: `getToolUiResourceUri(tool)` takes the v2 `Tool`
without a cast; `UiMetaShape` is replaced by ext-apps' own
`McpUiToolMeta` / `McpUiResourceMeta`, which resolve under NodeNext
since ext-apps#705. `AppInfo.csp` / `.permissions` / `.visibility`
take the named types too.
- core/mcp/inspectorClient.ts: `getAppRendererClient()` returns the SDK
client itself; the Proxy that translated the 1.x schema-first
`setNotificationHandler(Schema, handler)` into the method-string form,
its `notificationMethodFromSchema` helper and its cached field are
gone. `AppRendererClient` is now `Client` rather than `unknown`.
- createAppBridgeFactory.ts: `new AppBridge(client, HOST_INFO, …)` with
no `as unknown as`; `resolveSourceUri` passes the tool straight through.
- AppRenderer.tsx: `sendToolResult(result)` — its params ARE the v2
`CallToolResult` now.
- requestAppElicitation.ts: the double cast on the `elicitation/create`
send goes too — v2 `Protocol.request` takes the standard `Request`
shape, which `ElicitRequest` is. The helper itself stays: ext-apps#733
is not in 2.0.0.
- core/mcp/extensions.ts: `MCP_APP_MIME_TYPE` re-exports ext-apps'
`RESOURCE_MIME_TYPE` from `/app-bridge`; `UI_EXTENSION_KEY` stays
hardcoded because `EXTENSION_ID` is still `/server`-only in 2.0.0.
Tests: the Proxy-translation test is removed with the Proxy; the
`getAppRendererClient` tests assert the real `Client` instance and that
disconnect withdraws it; the `useInspectorClient` sentinel is a real
(unconnected) `Client`. Comments that cited ext-apps 1.7.5 now cite 2.0.0
where the fact still holds (#733 still unreleased).
Closes #1745
Refs #2309
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PsmUAZ6aLUAFqenjJKSSRp
Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
cliffhall
parent
84fccd024d
commit
a42d7fd99e
8 changed files with 127 additions and 242 deletions
|
|
@ -245,12 +245,7 @@ export function AppRenderer({
|
|||
if (pendingResultRef.current !== null) {
|
||||
const result = pendingResultRef.current;
|
||||
pendingResultRef.current = null;
|
||||
// ext-apps' AppBridge peers on SDK v1's CallToolResult (whose
|
||||
// `structuredContent` is typed narrower than v2's). Runtime-compatible;
|
||||
// cast at this boundary. TODO: drop when ext-apps#702 ships a v2 peer.
|
||||
void bridge.sendToolResult(
|
||||
result as Parameters<typeof bridge.sendToolResult>[0],
|
||||
);
|
||||
void bridge.sendToolResult(result);
|
||||
}
|
||||
}, []);
|
||||
|
||||
|
|
|
|||
|
|
@ -5,7 +5,7 @@ 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
|
||||
* ext-apps 2.0.0 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
|
||||
|
|
@ -25,10 +25,10 @@ const rawAppCapabilities = new WeakMap<AppBridge, Record<string, unknown>>();
|
|||
/**
|
||||
* 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
|
||||
* Structural, and generic over the message type, on purpose. This helper
|
||||
* touches one optional property, so naming the SDK `Transport` (which ext-apps'
|
||||
* `PostMessageTransport` implements since 2.0.0) would only make every test
|
||||
* fake carry `start`/`send`/`close` it never calls. 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
|
||||
|
|
|
|||
|
|
@ -39,9 +39,7 @@ import type { AppRenderSource, BridgeFactory } from "./AppRenderer";
|
|||
function resolveSourceUri(source: AppRenderSource): string | undefined {
|
||||
return source.kind === "resource"
|
||||
? source.resourceUri
|
||||
: getToolUiResourceUri(
|
||||
source.tool as Parameters<typeof getToolUiResourceUri>[0],
|
||||
);
|
||||
: getToolUiResourceUri(source.tool);
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -324,7 +322,7 @@ export function createAppBridgeFactory(
|
|||
// declare its own csp/permissions.
|
||||
const hostCapabilities: McpUiHostCapabilities = {
|
||||
...HOST_CAPABILITIES,
|
||||
// `elicitation` is not part of ext-apps 1.7.5's `McpUiHostCapabilities`
|
||||
// `elicitation` is not part of ext-apps 2.0.0'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
|
||||
|
|
@ -333,17 +331,9 @@ export function createAppBridgeFactory(
|
|||
? ({ elicitation: {} } as Partial<McpUiHostCapabilities>)
|
||||
: {}),
|
||||
};
|
||||
// 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.
|
||||
const bridge = new AppBridge(
|
||||
client as unknown as ConstructorParameters<typeof AppBridge>[0],
|
||||
HOST_INFO as unknown as ConstructorParameters<typeof AppBridge>[1],
|
||||
hostCapabilities,
|
||||
{
|
||||
hostContext: snapshotHostContext(iframe, HOST_AVAILABLE_DISPLAY_MODES),
|
||||
},
|
||||
);
|
||||
const bridge = new AppBridge(client, HOST_INFO, hostCapabilities, {
|
||||
hostContext: snapshotHostContext(iframe, HOST_AVAILABLE_DISPLAY_MODES),
|
||||
});
|
||||
|
||||
// The double-iframe proxy posts `sandboxready` once it can receive content.
|
||||
// Read the tool's UI resource and hand its HTML (plus any sandbox/permission
|
||||
|
|
|
|||
|
|
@ -21,7 +21,7 @@ export const APP_ELICITATION_TIMEOUT_MS = 10 * 60 * 1000;
|
|||
* 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
|
||||
* package (2.0.0) 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.
|
||||
*
|
||||
|
|
@ -37,23 +37,20 @@ export async function requestAppElicitation(
|
|||
// 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
|
||||
// NOT `bridge.getAppCapabilities()` directly: ext-apps 2.0.0 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<AppBridge["request"]>[0];
|
||||
return (await bridge.request(request, ElicitResultSchema, {
|
||||
// Since ext-apps 2.0.0 `AppBridge` extends the SDK v2 `Protocol`, whose
|
||||
// `request()` takes the standard `Request` shape — `ElicitRequest` is one —
|
||||
// so the send needs no cast (the 1.x peer's `AppRequest` union had no
|
||||
// `ElicitRequest` member, which is what forced a double cast here; #1745).
|
||||
// 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.
|
||||
const request: ElicitRequest = { method: "elicitation/create", params };
|
||||
return await bridge.request(request, ElicitResultSchema, {
|
||||
timeout: timeoutMs,
|
||||
})) as ElicitResult;
|
||||
});
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,5 +1,6 @@
|
|||
import { describe, it, expect } from "vitest";
|
||||
import { act, renderHook } from "@testing-library/react";
|
||||
import { Client } from "@modelcontextprotocol/client";
|
||||
import type {
|
||||
ClientCapabilities,
|
||||
Implementation,
|
||||
|
|
@ -422,7 +423,9 @@ describe("useInspectorClient", () => {
|
|||
const { result, rerender } = renderHook(() => useInspectorClient(client));
|
||||
expect(result.current.appRendererClient).toBeNull();
|
||||
|
||||
const sentinel = { iam: "renderer" };
|
||||
// A real (unconnected) SDK client: `AppRendererClient` is the SDK `Client`
|
||||
// itself since ext-apps 2.0.0 (#1745), and this hook only relays it.
|
||||
const sentinel = new Client({ name: "renderer-sentinel", version: "0" });
|
||||
client.setAppRendererClient(sentinel);
|
||||
rerender();
|
||||
expect(result.current.appRendererClient).toBe(sentinel);
|
||||
|
|
|
|||
|
|
@ -11,15 +11,19 @@ import {
|
|||
} from "@inspector/core/mcp/extensions.js";
|
||||
|
||||
/**
|
||||
* Drift guard for the MCP Apps UI advertisement (#1740). `MCP_APP_MIME_TYPE` is
|
||||
* hardcoded in core (ext-apps' constant lives on a `/server` subpath / an
|
||||
* extensionless re-export that doesn't resolve cleanly under NodeNext in the
|
||||
* browser build). A conforming server checks the client's advertised `mimeTypes`
|
||||
* before serving an App, so if the hardcoded string ever drifts from ext-apps'
|
||||
* real `RESOURCE_MIME_TYPE`, Apps silently stop working against strict servers.
|
||||
* Drift guard for the MCP Apps UI advertisement (#1740). `UI_EXTENSION_KEY` is
|
||||
* hardcoded in core (ext-apps' `EXTENSION_ID` still lives only on the `/server`
|
||||
* subpath as of 2.0.0, which the browser build must not import). A conforming
|
||||
* server keys its Apps lookup on the extension id and checks the client's
|
||||
* advertised `mimeTypes` before serving an App, so a drifted string silently
|
||||
* stops Apps working against strict servers.
|
||||
*
|
||||
* This runs in the node integration project, where importing the real ext-apps
|
||||
* value resolves — the one place the two can actually be compared.
|
||||
* `MCP_APP_MIME_TYPE` is no longer a copy — since ext-apps 2.0.0 core re-exports
|
||||
* `RESOURCE_MIME_TYPE` (#1745) — so its case here is a re-export check rather
|
||||
* than a drift check; it stays so the advertisement is asserted end to end.
|
||||
*
|
||||
* This runs in the node integration project, where importing the `/server`
|
||||
* value resolves — the one place the extension id can actually be compared.
|
||||
*/
|
||||
describe("MCP Apps UI extension constants (#1740)", () => {
|
||||
it("MCP_APP_MIME_TYPE matches ext-apps' RESOURCE_MIME_TYPE exactly", () => {
|
||||
|
|
|
|||
|
|
@ -24,7 +24,11 @@ import {
|
|||
createNumberedPrompts,
|
||||
} from "@modelcontextprotocol/inspector-test-server";
|
||||
import type { Tool } from "@modelcontextprotocol/client";
|
||||
import { ProtocolError, ProtocolErrorCode } from "@modelcontextprotocol/client";
|
||||
import {
|
||||
Client,
|
||||
ProtocolError,
|
||||
ProtocolErrorCode,
|
||||
} from "@modelcontextprotocol/client";
|
||||
|
||||
const serverCommand = getTestMcpServerCommand();
|
||||
|
||||
|
|
@ -114,26 +118,17 @@ describe("InspectorClient coverage backfill", () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe("getAppRendererClient proxy", () => {
|
||||
it("returns null before connect and a memoized proxy after connect", async () => {
|
||||
describe("getAppRendererClient", () => {
|
||||
it("returns null before connect and the SDK client after connect", async () => {
|
||||
client = stdioClient();
|
||||
// Not connected yet → null.
|
||||
expect(client.getAppRendererClient()).toBeNull();
|
||||
|
||||
await client.connect();
|
||||
const proxy = client.getAppRendererClient();
|
||||
expect(proxy).not.toBeNull();
|
||||
// Second call returns the same memoized proxy.
|
||||
expect(client.getAppRendererClient()).toBe(proxy);
|
||||
|
||||
// Accessing setNotificationHandler returns the wrapped function (covers
|
||||
// the prop === "setNotificationHandler" branch), and accessing another
|
||||
// prop returns the underlying value (covers the fall-through return).
|
||||
const wrapped = (proxy as unknown as { setNotificationHandler: unknown })
|
||||
.setNotificationHandler;
|
||||
expect(typeof wrapped).toBe("function");
|
||||
const other = (proxy as unknown as { request: unknown }).request;
|
||||
expect(typeof other).toBe("function");
|
||||
const appClient = client.getAppRendererClient();
|
||||
expect(appClient).toBeInstanceOf(Client);
|
||||
// Same instance on every call — it is the client, not a wrapper (#1745).
|
||||
expect(client.getAppRendererClient()).toBe(appClient);
|
||||
});
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -74,6 +74,7 @@ import type {
|
|||
ContentBlock,
|
||||
} from "@modelcontextprotocol/client";
|
||||
import {
|
||||
Client,
|
||||
LOG_LEVEL_META_KEY,
|
||||
RELATED_TASK_META_KEY,
|
||||
SdkError,
|
||||
|
|
@ -5661,7 +5662,7 @@ describe("InspectorClient", () => {
|
|||
});
|
||||
|
||||
describe("getAppRendererClient", () => {
|
||||
it("returns null before connect, and a cached proxy after connect", async () => {
|
||||
it("returns null before connect, and the SDK client itself after connect", async () => {
|
||||
server = createTestServerHttp({
|
||||
serverInfo: createTestServerInfo(),
|
||||
tools: [createEchoTool()],
|
||||
|
|
@ -5677,79 +5678,26 @@ describe("InspectorClient", () => {
|
|||
client = c;
|
||||
await c.connect();
|
||||
|
||||
const proxy1 = c.getAppRendererClient();
|
||||
expect(proxy1).not.toBeNull();
|
||||
// Second call returns the cached proxy
|
||||
expect(c.getAppRendererClient()).toBe(proxy1);
|
||||
expect(
|
||||
typeof (proxy1 as unknown as { setNotificationHandler?: unknown })
|
||||
.setNotificationHandler,
|
||||
).toBe("function");
|
||||
});
|
||||
|
||||
it("translates the ext-apps v1 schema-first setNotificationHandler call to v2's method-string form", async () => {
|
||||
// Regression: `@modelcontextprotocol/ext-apps` (SDK v1 peer) subscribes
|
||||
// with `setNotificationHandler(NotificationSchema, handler)`. On SDK v2
|
||||
// that throws "'[object Object]' is not a spec notification method",
|
||||
// breaking App rendering at connect. The proxy must translate the
|
||||
// schema (whose `.shape.method.value` is the method literal) to the
|
||||
// method string so the handler still fires on the real notification.
|
||||
server = createTestServerHttp({
|
||||
serverInfo: createTestServerInfo(),
|
||||
tools: [createAddToolTool()],
|
||||
listChanged: { tools: true },
|
||||
});
|
||||
await server.start();
|
||||
const c = new InspectorClient(
|
||||
{ type: "streamable-http", url: server.url },
|
||||
{ environment: { transport: createTransportNode } },
|
||||
);
|
||||
client = c;
|
||||
await c.connect();
|
||||
|
||||
const proxy = c.getAppRendererClient() as unknown as {
|
||||
setNotificationHandler: (
|
||||
schema: unknown,
|
||||
handler: () => void,
|
||||
) => void;
|
||||
};
|
||||
// A v1-style Zod notification schema, as ext-apps passes it (NOT a
|
||||
// string): its `.shape.method.value` carries the method literal.
|
||||
const v1StyleSchema = {
|
||||
shape: { method: { value: "notifications/tools/list_changed" } },
|
||||
};
|
||||
let handlerFired = false;
|
||||
const appClient = c.getAppRendererClient();
|
||||
expect(appClient).not.toBeNull();
|
||||
// The same instance on every call — no per-call wrapper. ext-apps 2.x
|
||||
// peers on SDK v2 and its `AppBridge` registers list-changed forwarding
|
||||
// with `setNotificationHandler("notifications/…", handler)`, so the
|
||||
// v1-peer translation proxy is gone (#1745).
|
||||
expect(c.getAppRendererClient()).toBe(appClient);
|
||||
expect(appClient).toBeInstanceOf(Client);
|
||||
// The string-first registration the bridge performs must land on it
|
||||
// directly and fire on the real notification.
|
||||
expect(() =>
|
||||
proxy.setNotificationHandler(v1StyleSchema, () => {
|
||||
handlerFired = true;
|
||||
}),
|
||||
).not.toThrow();
|
||||
|
||||
// The registration must land under the extracted method string: trigger
|
||||
// a real tools/list_changed and confirm the schema-first handler runs.
|
||||
const addToolTool = await getTool(c, "add_tool");
|
||||
await c.callTool(addToolTool, {
|
||||
name: "added_via_schema_first",
|
||||
description: "added at runtime",
|
||||
});
|
||||
await vi.waitFor(() => expect(handlerFired).toBe(true), {
|
||||
timeout: 5000,
|
||||
});
|
||||
|
||||
// A native string-first call (ours) passes through unchanged.
|
||||
expect(() =>
|
||||
proxy.setNotificationHandler(
|
||||
appClient?.setNotificationHandler(
|
||||
"notifications/prompts/list_changed",
|
||||
() => {},
|
||||
),
|
||||
).not.toThrow();
|
||||
|
||||
// An unrecognized first arg (no `.shape.method.value`) can't be
|
||||
// translated and falls through to the SDK, which rejects it clearly —
|
||||
// we don't silently swallow a genuinely-malformed registration.
|
||||
expect(() =>
|
||||
proxy.setNotificationHandler({} as unknown, () => {}),
|
||||
).toThrow(/not a spec notification method/);
|
||||
// Disconnecting withdraws it again.
|
||||
await c.disconnect();
|
||||
expect(c.getAppRendererClient()).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue