Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
98 changes: 97 additions & 1 deletion src/app-bridge.test.ts
Original file line number Diff line number Diff line change
@@ -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";
Expand Down Expand Up @@ -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" }];
Expand Down
37 changes: 37 additions & 0 deletions src/app-bridge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import {
ToolListChangedNotificationSchema,
} from "@modelcontextprotocol/sdk/types.js";
import {
Protocol,
ProtocolOptions,
RequestOptions,
} from "@modelcontextprotocol/sdk/shared/protocol.js";
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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) =>
Expand Down Expand Up @@ -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
Expand Down
51 changes: 50 additions & 1 deletion src/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
*
Expand All @@ -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<
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -851,6 +890,7 @@ export class App extends ProtocolWithEvents<
params: CallToolRequest["params"],
options?: RequestOptions,
): Promise<CallToolResult> {
this._assertInitialized("callServerTool");
if (typeof params === "string") {
throw new Error(
`callServerTool() expects an object as its first argument, but received a string ("${params}"). ` +
Expand Down Expand Up @@ -915,6 +955,7 @@ export class App extends ProtocolWithEvents<
params: ReadResourceRequest["params"],
options?: RequestOptions,
): Promise<ReadResourceResult> {
this._assertInitialized("readServerResource");
return await this.request(
{ method: "resources/read", params },
ReadResourceResultSchema,
Expand Down Expand Up @@ -962,6 +1003,7 @@ export class App extends ProtocolWithEvents<
params?: ListResourcesRequest["params"],
options?: RequestOptions,
): Promise<ListResourcesResult> {
this._assertInitialized("listServerResources");
return await this.request(
{ method: "resources/list", params },
ListResourcesResultSchema,
Expand Down Expand Up @@ -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(
<McpUiMessageRequest>{
method: "ui/message",
Expand Down Expand Up @@ -1113,6 +1156,7 @@ export class App extends ProtocolWithEvents<
params: McpUiUpdateModelContextRequest["params"],
options?: RequestOptions,
) {
this._assertInitialized("updateModelContext");
return this.request(
<McpUiUpdateModelContextRequest>{
method: "ui/update-model-context",
Expand Down Expand Up @@ -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(
<McpUiOpenLinkRequest>{
method: "ui/open-link",
Expand Down Expand Up @@ -1229,6 +1274,7 @@ export class App extends ProtocolWithEvents<
params: McpUiDownloadFileRequest["params"],
options?: RequestOptions,
) {
this._assertInitialized("downloadFile");
return this.request(
<McpUiDownloadFileRequest>{
method: "ui/download-file",
Expand Down Expand Up @@ -1313,6 +1359,7 @@ export class App extends ProtocolWithEvents<
params: McpUiRequestDisplayModeRequest["params"],
options?: RequestOptions,
) {
this._assertInitialized("requestDisplayMode");
return this.request(
<McpUiRequestDisplayModeRequest>{
method: "ui/request-display-mode",
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -1502,6 +1550,7 @@ export class App extends ProtocolWithEvents<
await this.notification(<McpUiInitializedNotification>{
method: "ui/notifications/initialized",
});
this._initializedSent = true;

if (this.options?.autoResize) {
this.setupSizeChangedNotifications();
Expand Down
21 changes: 15 additions & 6 deletions src/react/useApp.tsx
Original file line number Diff line number Diff line change
@@ -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<AppOptions, "strict"> {
/** App identification (name and version) */
appInfo: Implementation;
/**
Expand Down Expand Up @@ -121,6 +126,7 @@ export function useApp({
appInfo,
capabilities,
onAppCreated,
strict,
}: UseAppOptions): AppState {
const [app, setApp] = useState<App | null>(null);
const [isConnected, setIsConnected] = useState(false);
Expand All @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion typedoc.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ const config = {
],
excludePrivate: true,
excludeInternal: false,
intentionallyNotExported: ["AppOptions", "MethodSchema"],
intentionallyNotExported: ["MethodSchema"],
blockTags: [...OptionDefaults.blockTags, "@description"],
jsDocCompatibility: {
exampleTag: false,
Expand Down
Loading