Skip to content

Commit 737efe1

Browse files
ochafikochafik
committed
feat(app,app-bridge): guard requests sent before handshake completes (#620)
* feat(app,app-bridge): guard requests sent before handshake completes Calling host-bound methods before the ui/initialize → ui/notifications/initialized handshake completes can race the handshake on strict hosts and leave the iframe permanently hidden. App (View side): - All 8 host-bound methods (callServerTool, readServerResource, listServerResources, sendMessage, updateModelContext, openLink, downloadFile, requestDisplayMode) now check that connect() has sent the initialized notification. - New AppOptions.strict (default false): console.error when false, throw when true. AppOptions is now exported. - useApp() forwards strict to the App constructor. - Flag resets on reconnect. AppBridge (Host side): - replaceRequestHandler is overridden to wrap every host-bound handler with a console.warn if the request arrives before ui/notifications/initialized. Never throws. ui/initialize and ping use setRequestHandler directly and are exempt; notifications are unaffected. - Catches Views that hand-roll postMessage without the SDK. - Flag resets on reconnect. Refs anthropics/claude-ai-mcp#61, anthropics/claude-ai-mcp#149. * fix(docs): drop AppOptions from intentionallyNotExported now that it is exported
1 parent 739c0e0 commit 737efe1

5 files changed

Lines changed: 200 additions & 9 deletions

File tree

‎src/app-bridge.test.ts‎

Lines changed: 97 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { describe, it, expect, beforeEach, afterEach } from "bun:test";
1+
import { describe, it, expect, beforeEach, afterEach, spyOn } from "bun:test";
22
import { InMemoryTransport } from "@modelcontextprotocol/sdk/inMemory.js";
33
import type { Client } from "@modelcontextprotocol/sdk/client/index.js";
44
import type { ServerCapabilities } from "@modelcontextprotocol/sdk/types.js";
@@ -780,6 +780,102 @@ describe("App <-> AppBridge integration", () => {
780780
);
781781
});
782782

783+
describe("pre-handshake guard (claude-ai-mcp#149)", () => {
784+
const guardMsg =
785+
/called before connect\(\) completed the ui\/initialize handshake/;
786+
787+
it("callServerTool warns when called before connect() completes", async () => {
788+
const errSpy = spyOn(console, "error").mockImplementation(() => {});
789+
bridge.oncalltool = async () => ({ content: [] });
790+
await bridge.connect(bridgeTransport);
791+
792+
const connecting = app.connect(appTransport);
793+
// Handshake is in flight; _initializedSent is still false.
794+
await app.callServerTool({ name: "t", arguments: {} }).catch(() => {});
795+
796+
expect(errSpy).toHaveBeenCalledTimes(1);
797+
expect(errSpy.mock.calls[0][0]).toMatch(guardMsg);
798+
expect(errSpy.mock.calls[0][0]).toContain("App.callServerTool()");
799+
800+
await connecting;
801+
errSpy.mockRestore();
802+
});
803+
804+
it("callServerTool does not warn after connect() resolves", async () => {
805+
const errSpy = spyOn(console, "error").mockImplementation(() => {});
806+
bridge.oncalltool = async () => ({ content: [] });
807+
await bridge.connect(bridgeTransport);
808+
await app.connect(appTransport);
809+
810+
await app.callServerTool({ name: "t", arguments: {} });
811+
812+
expect(errSpy).not.toHaveBeenCalled();
813+
errSpy.mockRestore();
814+
});
815+
816+
it("sendMessage and readServerResource also warn before handshake", async () => {
817+
const errSpy = spyOn(console, "error").mockImplementation(() => {});
818+
819+
await app.sendMessage({ role: "user", content: [] }).catch(() => {});
820+
await app.readServerResource({ uri: "test://r" }).catch(() => {});
821+
822+
expect(errSpy).toHaveBeenCalledTimes(2);
823+
expect(errSpy.mock.calls[0][0]).toContain("App.sendMessage()");
824+
expect(errSpy.mock.calls[1][0]).toContain("App.readServerResource()");
825+
errSpy.mockRestore();
826+
});
827+
828+
it("throws instead of warning when strict: true", async () => {
829+
const errSpy = spyOn(console, "error").mockImplementation(() => {});
830+
const strictApp = new App(
831+
testAppInfo,
832+
{},
833+
{ autoResize: false, strict: true },
834+
);
835+
836+
await expect(
837+
strictApp.callServerTool({ name: "t", arguments: {} }),
838+
).rejects.toThrow(guardMsg);
839+
expect(errSpy).not.toHaveBeenCalled();
840+
841+
errSpy.mockRestore();
842+
});
843+
844+
it("AppBridge warns on tools/call from a View that skipped the handshake", async () => {
845+
const warnSpy = spyOn(console, "warn").mockImplementation(() => {});
846+
bridge.oncalltool = async () => ({ content: [] });
847+
await bridge.connect(bridgeTransport);
848+
849+
// Simulate a hand-rolled View (no SDK, no handshake) sending tools/call.
850+
await appTransport.start();
851+
appTransport.send({
852+
jsonrpc: "2.0",
853+
id: 1,
854+
method: "tools/call",
855+
params: { name: "t", arguments: {} },
856+
});
857+
await flush();
858+
859+
expect(warnSpy).toHaveBeenCalledTimes(1);
860+
expect(warnSpy.mock.calls[0][0]).toContain(
861+
"received 'tools/call' before ui/notifications/initialized",
862+
);
863+
warnSpy.mockRestore();
864+
});
865+
866+
it("AppBridge does not warn after initialized is received", async () => {
867+
const warnSpy = spyOn(console, "warn").mockImplementation(() => {});
868+
bridge.oncalltool = async () => ({ content: [] });
869+
await bridge.connect(bridgeTransport);
870+
await app.connect(appTransport);
871+
872+
await app.callServerTool({ name: "t", arguments: {} });
873+
874+
expect(warnSpy).not.toHaveBeenCalled();
875+
warnSpy.mockRestore();
876+
});
877+
});
878+
783879
it("onlistresources setter registers handler for resources/list requests", async () => {
784880
const requestParams = {};
785881
const resources = [{ uri: "test://resource", name: "Test" }];

‎src/app-bridge.ts‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ import {
3939
ToolListChangedNotificationSchema,
4040
} from "@modelcontextprotocol/sdk/types.js";
4141
import {
42+
Protocol,
4243
ProtocolOptions,
4344
RequestOptions,
4445
} from "@modelcontextprotocol/sdk/shared/protocol.js";
@@ -307,6 +308,37 @@ export class AppBridge extends ProtocolWithEvents<
307308
private _appCapabilities?: McpUiAppCapabilities;
308309
private _hostContext: McpUiHostContext = {};
309310
private _appInfo?: Implementation;
311+
private _initializedReceived = false;
312+
313+
/**
314+
* Wrap every handler registered via `replaceRequestHandler` with a check
315+
* that the View has sent `ui/notifications/initialized`. Warns (never
316+
* throws) so lenient hosts keep working while still surfacing the
317+
* misordering that leaves strict hosts with a permanently hidden iframe.
318+
* `ui/initialize` and `ping` use `setRequestHandler` directly and are
319+
* intentionally exempt.
320+
*
321+
* @see {@link https://github.com/anthropics/claude-ai-mcp/issues/149 claude-ai-mcp#149}
322+
*/
323+
private _baseReplaceRequestHandler = this.replaceRequestHandler;
324+
protected override replaceRequestHandler: Protocol<
325+
AppRequest,
326+
AppNotification,
327+
AppResult
328+
>["setRequestHandler"] = (schema, handler) => {
329+
this._baseReplaceRequestHandler(schema, (request, extra) => {
330+
if (!this._initializedReceived) {
331+
console.warn(
332+
`[ext-apps] AppBridge received '${request.method}' before ` +
333+
`ui/notifications/initialized. The View is calling host ` +
334+
`methods before completing the handshake; it should await ` +
335+
`app.connect() first. See ` +
336+
`https://github.com/anthropics/claude-ai-mcp/issues/149`,
337+
);
338+
}
339+
return handler(request, extra);
340+
});
341+
};
310342

311343
protected readonly eventSchemas = {
312344
sizechange: McpUiSizeChangedNotificationSchema,
@@ -357,6 +389,10 @@ export class AppBridge extends ProtocolWithEvents<
357389
) {
358390
super(options);
359391

392+
this.addEventListener("initialized", () => {
393+
this._initializedReceived = true;
394+
});
395+
360396
this._hostContext = options?.hostContext || {};
361397

362398
this.setRequestHandler(McpUiInitializeRequestSchema, (request) =>
@@ -1758,6 +1794,7 @@ export class AppBridge extends ProtocolWithEvents<
17581794
"AppBridge is already connected. Call close() before connecting again.",
17591795
);
17601796
}
1797+
this._initializedReceived = false;
17611798
if (this._client) {
17621799
// When a client was passed to the constructor, automatically forward
17631800
// MCP requests/notifications between the view and the server

‎src/app.ts‎

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ export const RESOURCE_MIME_TYPE = "text/html;profile=mcp-app";
144144
*
145145
* @see `ProtocolOptions` from @modelcontextprotocol/sdk for inherited options
146146
*/
147-
type AppOptions = ProtocolOptions & {
147+
export type AppOptions = ProtocolOptions & {
148148
/**
149149
* Automatically report size changes to the host using `ResizeObserver`.
150150
*
@@ -155,6 +155,19 @@ type AppOptions = ProtocolOptions & {
155155
* @default true
156156
*/
157157
autoResize?: boolean;
158+
/**
159+
* Throw on detected misuse instead of logging a console error.
160+
*
161+
* Currently this affects calling host-bound methods (e.g.
162+
* {@link App.callServerTool `callServerTool`}, {@link App.sendMessage `sendMessage`})
163+
* before {@link App.connect `connect`} has completed the `ui/initialize`
164+
* handshake. With `strict: false` (default) a `console.error` is emitted;
165+
* with `strict: true` an `Error` is thrown.
166+
*
167+
* @remarks Throwing will become the default in a future release.
168+
* @default false
169+
*/
170+
strict?: boolean;
158171
};
159172

160173
type RequestHandlerExtra = Parameters<
@@ -244,6 +257,32 @@ export class App extends ProtocolWithEvents<
244257
private _hostCapabilities?: McpUiHostCapabilities;
245258
private _hostInfo?: Implementation;
246259
private _hostContext?: McpUiHostContext;
260+
private _initializedSent = false;
261+
262+
/**
263+
* Warn if a host-bound method is called before {@link connect `connect`} has
264+
* completed the `ui/initialize` → `ui/notifications/initialized` handshake.
265+
*
266+
* Calling these methods early can race the handshake on strict hosts and
267+
* leave the iframe permanently hidden. See
268+
* {@link https://github.com/anthropics/claude-ai-mcp/issues/61 claude-ai-mcp#61} /
269+
* {@link https://github.com/anthropics/claude-ai-mcp/issues/149 #149}.
270+
*
271+
* @remarks This will become a thrown `Error` in a future minor release.
272+
*/
273+
private _assertInitialized(method: string): void {
274+
if (this._initializedSent) return;
275+
const msg =
276+
`[ext-apps] App.${method}() called before connect() completed the ` +
277+
`ui/initialize handshake. Await app.connect() before calling this ` +
278+
`method, or move data loading to an ontoolresult handler. ` +
279+
`See https://github.com/anthropics/claude-ai-mcp/issues/149`;
280+
if (this.options?.strict) {
281+
throw new Error(msg);
282+
}
283+
// TODO(next-minor): make `strict: true` the default.
284+
console.error(`${msg}. This will throw in a future release.`);
285+
}
247286

248287
protected readonly eventSchemas = {
249288
toolinput: McpUiToolInputNotificationSchema,
@@ -851,6 +890,7 @@ export class App extends ProtocolWithEvents<
851890
params: CallToolRequest["params"],
852891
options?: RequestOptions,
853892
): Promise<CallToolResult> {
893+
this._assertInitialized("callServerTool");
854894
if (typeof params === "string") {
855895
throw new Error(
856896
`callServerTool() expects an object as its first argument, but received a string ("${params}"). ` +
@@ -915,6 +955,7 @@ export class App extends ProtocolWithEvents<
915955
params: ReadResourceRequest["params"],
916956
options?: RequestOptions,
917957
): Promise<ReadResourceResult> {
958+
this._assertInitialized("readServerResource");
918959
return await this.request(
919960
{ method: "resources/read", params },
920961
ReadResourceResultSchema,
@@ -962,6 +1003,7 @@ export class App extends ProtocolWithEvents<
9621003
params?: ListResourcesRequest["params"],
9631004
options?: RequestOptions,
9641005
): Promise<ListResourcesResult> {
1006+
this._assertInitialized("listServerResources");
9651007
return await this.request(
9661008
{ method: "resources/list", params },
9671009
ListResourcesResultSchema,
@@ -1020,6 +1062,7 @@ export class App extends ProtocolWithEvents<
10201062
* @see {@link McpUiMessageRequest `McpUiMessageRequest`} for request structure
10211063
*/
10221064
sendMessage(params: McpUiMessageRequest["params"], options?: RequestOptions) {
1065+
this._assertInitialized("sendMessage");
10231066
return this.request(
10241067
<McpUiMessageRequest>{
10251068
method: "ui/message",
@@ -1113,6 +1156,7 @@ export class App extends ProtocolWithEvents<
11131156
params: McpUiUpdateModelContextRequest["params"],
11141157
options?: RequestOptions,
11151158
) {
1159+
this._assertInitialized("updateModelContext");
11161160
return this.request(
11171161
<McpUiUpdateModelContextRequest>{
11181162
method: "ui/update-model-context",
@@ -1149,6 +1193,7 @@ export class App extends ProtocolWithEvents<
11491193
* @see {@link McpUiOpenLinkResult `McpUiOpenLinkResult`} for result structure
11501194
*/
11511195
openLink(params: McpUiOpenLinkRequest["params"], options?: RequestOptions) {
1196+
this._assertInitialized("openLink");
11521197
return this.request(
11531198
<McpUiOpenLinkRequest>{
11541199
method: "ui/open-link",
@@ -1229,6 +1274,7 @@ export class App extends ProtocolWithEvents<
12291274
params: McpUiDownloadFileRequest["params"],
12301275
options?: RequestOptions,
12311276
) {
1277+
this._assertInitialized("downloadFile");
12321278
return this.request(
12331279
<McpUiDownloadFileRequest>{
12341280
method: "ui/download-file",
@@ -1313,6 +1359,7 @@ export class App extends ProtocolWithEvents<
13131359
params: McpUiRequestDisplayModeRequest["params"],
13141360
options?: RequestOptions,
13151361
) {
1362+
this._assertInitialized("requestDisplayMode");
13161363
return this.request(
13171364
<McpUiRequestDisplayModeRequest>{
13181365
method: "ui/request-display-mode",
@@ -1475,6 +1522,7 @@ export class App extends ProtocolWithEvents<
14751522
"App is already connected. Call close() before connecting again.",
14761523
);
14771524
}
1525+
this._initializedSent = false;
14781526
await super.connect(transport);
14791527

14801528
try {
@@ -1502,6 +1550,7 @@ export class App extends ProtocolWithEvents<
15021550
await this.notification(<McpUiInitializedNotification>{
15031551
method: "ui/notifications/initialized",
15041552
});
1553+
this._initializedSent = true;
15051554

15061555
if (this.options?.autoResize) {
15071556
this.setupSizeChangedNotifications();

‎src/react/useApp.tsx‎

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,25 @@
11
import { useEffect, useState } from "react";
22
import { Implementation } from "@modelcontextprotocol/sdk/types.js";
33
import { Client } from "@modelcontextprotocol/sdk/client";
4-
import { App, McpUiAppCapabilities, PostMessageTransport } from "../app";
4+
import {
5+
App,
6+
AppOptions,
7+
McpUiAppCapabilities,
8+
PostMessageTransport,
9+
} from "../app";
510
export * from "../app";
611

712
/**
813
* Options for configuring the {@link useApp `useApp`} hook.
914
*
10-
* Note: This interface does NOT expose {@link App `App`} options like `autoResize`.
11-
* The hook creates the `App` with default options (`autoResize: true`). If you
12-
* need custom `App` options, create the `App` manually instead of using this hook.
15+
* The `strict` option is forwarded to the underlying {@link App `App`}
16+
* instance. For other {@link AppOptions `AppOptions`}, create the `App`
17+
* manually instead of using this hook.
1318
*
1419
* @see {@link useApp `useApp`} for the hook that uses these options
1520
* @see {@link useAutoResize `useAutoResize`} for manual auto-resize control with custom `App` options
1621
*/
17-
export interface UseAppOptions {
22+
export interface UseAppOptions extends Pick<AppOptions, "strict"> {
1823
/** App identification (name and version) */
1924
appInfo: Implementation;
2025
/**
@@ -121,6 +126,7 @@ export function useApp({
121126
appInfo,
122127
capabilities,
123128
onAppCreated,
129+
strict,
124130
}: UseAppOptions): AppState {
125131
const [app, setApp] = useState<App | null>(null);
126132
const [isConnected, setIsConnected] = useState(false);
@@ -135,7 +141,10 @@ export function useApp({
135141
window.parent,
136142
window.parent,
137143
);
138-
const app = new App(appInfo, capabilities);
144+
const app = new App(appInfo, capabilities, {
145+
autoResize: true,
146+
strict,
147+
});
139148

140149
// Register handlers BEFORE connecting
141150
onAppCreated?.(app);

‎typedoc.config.mjs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ const config = {
2828
],
2929
excludePrivate: true,
3030
excludeInternal: false,
31-
intentionallyNotExported: ["AppOptions", "MethodSchema"],
31+
intentionallyNotExported: ["MethodSchema"],
3232
blockTags: [...OptionDefaults.blockTags, "@description"],
3333
jsDocCompatibility: {
3434
exampleTag: false,

0 commit comments

Comments
 (0)