diff --git a/src/app-bridge.test.ts b/src/app-bridge.test.ts index 4761d766a..e2e471b72 100644 --- a/src/app-bridge.test.ts +++ b/src/app-bridge.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect, beforeEach, afterEach } from "bun:test"; +import { describe, it, expect, beforeEach, afterEach, spyOn } from "bun:test"; import { InMemoryTransport } from "@modelcontextprotocol/sdk/inMemory.js"; import type { Client } from "@modelcontextprotocol/sdk/client/index.js"; import type { ServerCapabilities } from "@modelcontextprotocol/sdk/types.js"; @@ -780,6 +780,102 @@ describe("App <-> AppBridge integration", () => { ); }); + describe("pre-handshake guard (claude-ai-mcp#149)", () => { + const guardMsg = + /called before connect\(\) completed the ui\/initialize handshake/; + + it("callServerTool warns when called before connect() completes", async () => { + const errSpy = spyOn(console, "error").mockImplementation(() => {}); + bridge.oncalltool = async () => ({ content: [] }); + await bridge.connect(bridgeTransport); + + const connecting = app.connect(appTransport); + // Handshake is in flight; _initializedSent is still false. + await app.callServerTool({ name: "t", arguments: {} }).catch(() => {}); + + expect(errSpy).toHaveBeenCalledTimes(1); + expect(errSpy.mock.calls[0][0]).toMatch(guardMsg); + expect(errSpy.mock.calls[0][0]).toContain("App.callServerTool()"); + + await connecting; + errSpy.mockRestore(); + }); + + it("callServerTool does not warn after connect() resolves", async () => { + const errSpy = spyOn(console, "error").mockImplementation(() => {}); + bridge.oncalltool = async () => ({ content: [] }); + await bridge.connect(bridgeTransport); + await app.connect(appTransport); + + await app.callServerTool({ name: "t", arguments: {} }); + + expect(errSpy).not.toHaveBeenCalled(); + errSpy.mockRestore(); + }); + + it("sendMessage and readServerResource also warn before handshake", async () => { + const errSpy = spyOn(console, "error").mockImplementation(() => {}); + + await app.sendMessage({ role: "user", content: [] }).catch(() => {}); + await app.readServerResource({ uri: "test://r" }).catch(() => {}); + + expect(errSpy).toHaveBeenCalledTimes(2); + expect(errSpy.mock.calls[0][0]).toContain("App.sendMessage()"); + expect(errSpy.mock.calls[1][0]).toContain("App.readServerResource()"); + errSpy.mockRestore(); + }); + + it("throws instead of warning when strict: true", async () => { + const errSpy = spyOn(console, "error").mockImplementation(() => {}); + const strictApp = new App( + testAppInfo, + {}, + { autoResize: false, strict: true }, + ); + + await expect( + strictApp.callServerTool({ name: "t", arguments: {} }), + ).rejects.toThrow(guardMsg); + expect(errSpy).not.toHaveBeenCalled(); + + errSpy.mockRestore(); + }); + + it("AppBridge warns on tools/call from a View that skipped the handshake", async () => { + const warnSpy = spyOn(console, "warn").mockImplementation(() => {}); + bridge.oncalltool = async () => ({ content: [] }); + await bridge.connect(bridgeTransport); + + // Simulate a hand-rolled View (no SDK, no handshake) sending tools/call. + await appTransport.start(); + appTransport.send({ + jsonrpc: "2.0", + id: 1, + method: "tools/call", + params: { name: "t", arguments: {} }, + }); + await flush(); + + expect(warnSpy).toHaveBeenCalledTimes(1); + expect(warnSpy.mock.calls[0][0]).toContain( + "received 'tools/call' before ui/notifications/initialized", + ); + warnSpy.mockRestore(); + }); + + it("AppBridge does not warn after initialized is received", async () => { + const warnSpy = spyOn(console, "warn").mockImplementation(() => {}); + bridge.oncalltool = async () => ({ content: [] }); + await bridge.connect(bridgeTransport); + await app.connect(appTransport); + + await app.callServerTool({ name: "t", arguments: {} }); + + expect(warnSpy).not.toHaveBeenCalled(); + warnSpy.mockRestore(); + }); + }); + it("onlistresources setter registers handler for resources/list requests", async () => { const requestParams = {}; const resources = [{ uri: "test://resource", name: "Test" }]; diff --git a/src/app-bridge.ts b/src/app-bridge.ts index 77125872c..3c616185e 100644 --- a/src/app-bridge.ts +++ b/src/app-bridge.ts @@ -39,6 +39,7 @@ import { ToolListChangedNotificationSchema, } from "@modelcontextprotocol/sdk/types.js"; import { + Protocol, ProtocolOptions, RequestOptions, } from "@modelcontextprotocol/sdk/shared/protocol.js"; @@ -307,6 +308,37 @@ export class AppBridge extends ProtocolWithEvents< private _appCapabilities?: McpUiAppCapabilities; private _hostContext: McpUiHostContext = {}; private _appInfo?: Implementation; + private _initializedReceived = false; + + /** + * Wrap every handler registered via `replaceRequestHandler` with a check + * that the View has sent `ui/notifications/initialized`. Warns (never + * throws) so lenient hosts keep working while still surfacing the + * misordering that leaves strict hosts with a permanently hidden iframe. + * `ui/initialize` and `ping` use `setRequestHandler` directly and are + * intentionally exempt. + * + * @see {@link https://github.com/anthropics/claude-ai-mcp/issues/149 claude-ai-mcp#149} + */ + private _baseReplaceRequestHandler = this.replaceRequestHandler; + protected override replaceRequestHandler: Protocol< + AppRequest, + AppNotification, + AppResult + >["setRequestHandler"] = (schema, handler) => { + this._baseReplaceRequestHandler(schema, (request, extra) => { + if (!this._initializedReceived) { + console.warn( + `[ext-apps] AppBridge received '${request.method}' before ` + + `ui/notifications/initialized. The View is calling host ` + + `methods before completing the handshake; it should await ` + + `app.connect() first. See ` + + `https://github.com/anthropics/claude-ai-mcp/issues/149`, + ); + } + return handler(request, extra); + }); + }; protected readonly eventSchemas = { sizechange: McpUiSizeChangedNotificationSchema, @@ -357,6 +389,10 @@ export class AppBridge extends ProtocolWithEvents< ) { super(options); + this.addEventListener("initialized", () => { + this._initializedReceived = true; + }); + this._hostContext = options?.hostContext || {}; this.setRequestHandler(McpUiInitializeRequestSchema, (request) => @@ -1758,6 +1794,7 @@ export class AppBridge extends ProtocolWithEvents< "AppBridge is already connected. Call close() before connecting again.", ); } + this._initializedReceived = false; if (this._client) { // When a client was passed to the constructor, automatically forward // MCP requests/notifications between the view and the server diff --git a/src/app.ts b/src/app.ts index 4f429e829..fa4844026 100644 --- a/src/app.ts +++ b/src/app.ts @@ -144,7 +144,7 @@ export const RESOURCE_MIME_TYPE = "text/html;profile=mcp-app"; * * @see `ProtocolOptions` from @modelcontextprotocol/sdk for inherited options */ -type AppOptions = ProtocolOptions & { +export type AppOptions = ProtocolOptions & { /** * Automatically report size changes to the host using `ResizeObserver`. * @@ -155,6 +155,19 @@ type AppOptions = ProtocolOptions & { * @default true */ autoResize?: boolean; + /** + * Throw on detected misuse instead of logging a console error. + * + * Currently this affects calling host-bound methods (e.g. + * {@link App.callServerTool `callServerTool`}, {@link App.sendMessage `sendMessage`}) + * before {@link App.connect `connect`} has completed the `ui/initialize` + * handshake. With `strict: false` (default) a `console.error` is emitted; + * with `strict: true` an `Error` is thrown. + * + * @remarks Throwing will become the default in a future release. + * @default false + */ + strict?: boolean; }; type RequestHandlerExtra = Parameters< @@ -244,6 +257,32 @@ export class App extends ProtocolWithEvents< private _hostCapabilities?: McpUiHostCapabilities; private _hostInfo?: Implementation; private _hostContext?: McpUiHostContext; + private _initializedSent = false; + + /** + * Warn if a host-bound method is called before {@link connect `connect`} has + * completed the `ui/initialize` → `ui/notifications/initialized` handshake. + * + * Calling these methods early can race the handshake on strict hosts and + * leave the iframe permanently hidden. See + * {@link https://github.com/anthropics/claude-ai-mcp/issues/61 claude-ai-mcp#61} / + * {@link https://github.com/anthropics/claude-ai-mcp/issues/149 #149}. + * + * @remarks This will become a thrown `Error` in a future minor release. + */ + private _assertInitialized(method: string): void { + if (this._initializedSent) return; + const msg = + `[ext-apps] App.${method}() called before connect() completed the ` + + `ui/initialize handshake. Await app.connect() before calling this ` + + `method, or move data loading to an ontoolresult handler. ` + + `See https://github.com/anthropics/claude-ai-mcp/issues/149`; + if (this.options?.strict) { + throw new Error(msg); + } + // TODO(next-minor): make `strict: true` the default. + console.error(`${msg}. This will throw in a future release.`); + } protected readonly eventSchemas = { toolinput: McpUiToolInputNotificationSchema, @@ -851,6 +890,7 @@ export class App extends ProtocolWithEvents< params: CallToolRequest["params"], options?: RequestOptions, ): Promise { + this._assertInitialized("callServerTool"); if (typeof params === "string") { throw new Error( `callServerTool() expects an object as its first argument, but received a string ("${params}"). ` + @@ -915,6 +955,7 @@ export class App extends ProtocolWithEvents< params: ReadResourceRequest["params"], options?: RequestOptions, ): Promise { + this._assertInitialized("readServerResource"); return await this.request( { method: "resources/read", params }, ReadResourceResultSchema, @@ -962,6 +1003,7 @@ export class App extends ProtocolWithEvents< params?: ListResourcesRequest["params"], options?: RequestOptions, ): Promise { + this._assertInitialized("listServerResources"); return await this.request( { method: "resources/list", params }, ListResourcesResultSchema, @@ -1020,6 +1062,7 @@ export class App extends ProtocolWithEvents< * @see {@link McpUiMessageRequest `McpUiMessageRequest`} for request structure */ sendMessage(params: McpUiMessageRequest["params"], options?: RequestOptions) { + this._assertInitialized("sendMessage"); return this.request( { method: "ui/message", @@ -1113,6 +1156,7 @@ export class App extends ProtocolWithEvents< params: McpUiUpdateModelContextRequest["params"], options?: RequestOptions, ) { + this._assertInitialized("updateModelContext"); return this.request( { method: "ui/update-model-context", @@ -1149,6 +1193,7 @@ export class App extends ProtocolWithEvents< * @see {@link McpUiOpenLinkResult `McpUiOpenLinkResult`} for result structure */ openLink(params: McpUiOpenLinkRequest["params"], options?: RequestOptions) { + this._assertInitialized("openLink"); return this.request( { method: "ui/open-link", @@ -1229,6 +1274,7 @@ export class App extends ProtocolWithEvents< params: McpUiDownloadFileRequest["params"], options?: RequestOptions, ) { + this._assertInitialized("downloadFile"); return this.request( { method: "ui/download-file", @@ -1313,6 +1359,7 @@ export class App extends ProtocolWithEvents< params: McpUiRequestDisplayModeRequest["params"], options?: RequestOptions, ) { + this._assertInitialized("requestDisplayMode"); return this.request( { method: "ui/request-display-mode", @@ -1475,6 +1522,7 @@ export class App extends ProtocolWithEvents< "App is already connected. Call close() before connecting again.", ); } + this._initializedSent = false; await super.connect(transport); try { @@ -1502,6 +1550,7 @@ export class App extends ProtocolWithEvents< await this.notification({ method: "ui/notifications/initialized", }); + this._initializedSent = true; if (this.options?.autoResize) { this.setupSizeChangedNotifications(); diff --git a/src/react/useApp.tsx b/src/react/useApp.tsx index d6dcfbb79..75b3e15f0 100644 --- a/src/react/useApp.tsx +++ b/src/react/useApp.tsx @@ -1,20 +1,25 @@ import { useEffect, useState } from "react"; import { Implementation } from "@modelcontextprotocol/sdk/types.js"; import { Client } from "@modelcontextprotocol/sdk/client"; -import { App, McpUiAppCapabilities, PostMessageTransport } from "../app"; +import { + App, + AppOptions, + McpUiAppCapabilities, + PostMessageTransport, +} from "../app"; export * from "../app"; /** * Options for configuring the {@link useApp `useApp`} hook. * - * Note: This interface does NOT expose {@link App `App`} options like `autoResize`. - * The hook creates the `App` with default options (`autoResize: true`). If you - * need custom `App` options, create the `App` manually instead of using this hook. + * The `strict` option is forwarded to the underlying {@link App `App`} + * instance. For other {@link AppOptions `AppOptions`}, create the `App` + * manually instead of using this hook. * * @see {@link useApp `useApp`} for the hook that uses these options * @see {@link useAutoResize `useAutoResize`} for manual auto-resize control with custom `App` options */ -export interface UseAppOptions { +export interface UseAppOptions extends Pick { /** App identification (name and version) */ appInfo: Implementation; /** @@ -121,6 +126,7 @@ export function useApp({ appInfo, capabilities, onAppCreated, + strict, }: UseAppOptions): AppState { const [app, setApp] = useState(null); const [isConnected, setIsConnected] = useState(false); @@ -135,7 +141,10 @@ export function useApp({ window.parent, window.parent, ); - const app = new App(appInfo, capabilities); + const app = new App(appInfo, capabilities, { + autoResize: true, + strict, + }); // Register handlers BEFORE connecting onAppCreated?.(app); diff --git a/typedoc.config.mjs b/typedoc.config.mjs index 5775caec7..793737cee 100644 --- a/typedoc.config.mjs +++ b/typedoc.config.mjs @@ -28,7 +28,7 @@ const config = { ], excludePrivate: true, excludeInternal: false, - intentionallyNotExported: ["AppOptions", "MethodSchema"], + intentionallyNotExported: ["MethodSchema"], blockTags: [...OptionDefaults.blockTags, "@description"], jsDocCompatibility: { exampleTag: false,