Fix MCP App load regression: translate ext-apps v1 setNotificationHandler API to v2
An MCP App (e.g. NightRider's drum-pad) failed to load with "'[object Object]' is not a spec notification method; pass schemas as the second argument to setNotificationHandler()". `@modelcontextprotocol/ext-apps` still peers on SDK v1 and, during the AppBridge connect handshake, subscribes to tools/resources/prompts list_changed with the v1 schema-first API `setNotificationHandler(NotificationSchema, handler)`. SDK v2 requires a method STRING and throws on a schema object — the migration's compat shim covered the types but not this runtime API difference. Fix in the app-renderer client Proxy (`getAppRendererClient`): translate a schema-first `setNotificationHandler` call to v2's method-string form by reading the method literal off the schema's `shape.method.value`; native string-first calls pass through, and an unrecognized first arg falls through to the SDK's clear error rather than being swallowed. Only `setNotificationHandler` needed this — AppBridge's `request(req, schema, …)` calls work as-is (the v1 SDK schemas expose `~standard`, so v2 accepts them). Remove when ext-apps#702 ships a v2 peer. Added an integration test covering the schema-first translation end-to-end (handler fires on a real tools/list_changed), plus the string-passthrough and unrecognized-schema branches. Coverage ≥90 holds; typecheck + App tests green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
f9f79fff54
commit
8a7a38f800
2 changed files with 103 additions and 5 deletions
|
|
@ -5157,7 +5157,7 @@ describe("InspectorClient", () => {
|
|||
});
|
||||
|
||||
describe("getAppRendererClient", () => {
|
||||
it("returns null before connect, and a proxy after connect that forwards setNotificationHandler", async () => {
|
||||
it("returns null before connect, and a cached proxy after connect", async () => {
|
||||
server = createTestServerHttp({
|
||||
serverInfo: createTestServerInfo(),
|
||||
tools: [createEchoTool()],
|
||||
|
|
@ -5177,12 +5177,76 @@ describe("InspectorClient", () => {
|
|||
expect(proxy1).not.toBeNull();
|
||||
// Second call returns the cached proxy
|
||||
expect(c.getAppRendererClient()).toBe(proxy1);
|
||||
// setNotificationHandler on the proxy delegates to the underlying client
|
||||
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;
|
||||
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(
|
||||
"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/);
|
||||
});
|
||||
});
|
||||
|
||||
describe("list_changed + resourceUpdated notifications", () => {
|
||||
|
|
|
|||
|
|
@ -186,6 +186,24 @@ const TOOL_CALL_CANCELLED_REASON = "Tool call cancelled by user";
|
|||
*/
|
||||
const DEFAULT_TASK_POLL_INTERVAL_MS = 500;
|
||||
|
||||
/**
|
||||
* Extract the method literal from an MCP notification Zod schema (e.g.
|
||||
* `ToolListChangedNotificationSchema`), or `undefined` if the shape isn't
|
||||
* recognized. Used by the App-renderer client proxy to translate the SDK-v1
|
||||
* schema-first `setNotificationHandler` API — which `@modelcontextprotocol/ext-apps`
|
||||
* still uses — into SDK v2's method-string form. Reads the `method` literal off
|
||||
* the notification schema's `shape` (the shape both the v1 SDK and v2 core
|
||||
* schemas expose).
|
||||
*/
|
||||
function notificationMethodFromSchema(schema: unknown): string | undefined {
|
||||
if (schema !== null && typeof schema === "object") {
|
||||
const literal = (schema as { shape?: { method?: { value?: unknown } } })
|
||||
.shape?.method?.value;
|
||||
if (typeof literal === "string") return literal;
|
||||
}
|
||||
return undefined;
|
||||
}
|
||||
|
||||
/**
|
||||
* The descriptor for a single tools/call, threaded through the retry loop and
|
||||
* each attempt. Bundled into one object so `callToolWithRetries`/`attemptToolCall`
|
||||
|
|
@ -1441,9 +1459,25 @@ export class InspectorClient extends InspectorClientEventTarget {
|
|||
get(proxyTarget, prop, receiver) {
|
||||
const value = Reflect.get(proxyTarget, prop, receiver);
|
||||
if (prop === "setNotificationHandler" && typeof value === "function") {
|
||||
return (...args: Parameters<Client["setNotificationHandler"]>) => {
|
||||
// Add behavior here (e.g. wrap handler, log, filter)
|
||||
return value.apply(target, args);
|
||||
return (schemaOrMethod: unknown, ...rest: unknown[]) => {
|
||||
// `@modelcontextprotocol/ext-apps` still peers on SDK v1 and
|
||||
// subscribes to list-changed notifications with the v1 schema-first
|
||||
// API `setNotificationHandler(NotificationSchema, handler)`. SDK v2
|
||||
// requires a method STRING as the first argument and throws
|
||||
// "'[object Object]' is not a spec notification method" on a schema —
|
||||
// which broke App rendering during the initial connect handshake.
|
||||
// Translate a schema-first call to the method-string form; native
|
||||
// string-first calls (ours) pass through untouched. Remove when
|
||||
// ext-apps#702 ships a v2 peer.
|
||||
const method =
|
||||
typeof schemaOrMethod === "string"
|
||||
? schemaOrMethod
|
||||
: (notificationMethodFromSchema(schemaOrMethod) ??
|
||||
schemaOrMethod);
|
||||
return (value as (...a: unknown[]) => unknown).apply(target, [
|
||||
method,
|
||||
...rest,
|
||||
]);
|
||||
};
|
||||
}
|
||||
return value;
|
||||
|
|
|
|||
Loading…
Reference in a new issue