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
10 changes: 8 additions & 2 deletions sdks/typescript/client/src/components/AppFrame.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {
type McpUiAppCapabilities,
} from '@modelcontextprotocol/ext-apps/app-bridge';

import { setupSandboxProxyIframe } from '../utils/app-host-utils';
import { setupSandboxProxyIframe, type SandboxCancelRef } from '../utils/app-host-utils';

/**
* Build sandbox URL with CSP query parameter for HTTP header-based CSP enforcement.
Expand Down Expand Up @@ -165,6 +165,11 @@ export const AppFrame = (props: AppFrameProps) => {
setError(null);

let mounted = true;
// cancelRef is populated synchronously inside setupSandboxProxyIframe's Promise
// constructor, before any async yield. This guarantees it is set by the time
// React Strict Mode fires the effect cleanup — allowing the orphaned timeout
// to be cleared even when cleanup runs before the awaited result resolves.
const cancelRef: SandboxCancelRef = {};

const setup = async () => {
try {
Expand All @@ -176,7 +181,7 @@ export const AppFrame = (props: AppFrameProps) => {
currentAppBridgeRef.current = null;
}

const { iframe, onReady } = await setupSandboxProxyIframe(sandboxUrl);
const { iframe, onReady } = await setupSandboxProxyIframe(sandboxUrl, cancelRef);

if (!mounted) return;

Expand Down Expand Up @@ -237,6 +242,7 @@ export const AppFrame = (props: AppFrameProps) => {

return () => {
mounted = false;
cancelRef.cancel?.();
};
}, [sandbox.url, sandbox.csp, appBridge]);

Expand Down
43 changes: 41 additions & 2 deletions sdks/typescript/client/src/components/__tests__/AppFrame.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,10 @@ describe('<AppFrame />', () => {
render(<AppFrame {...getPropsWithBridge()} />);

await waitFor(() => {
expect(appHostUtils.setupSandboxProxyIframe).toHaveBeenCalledWith(defaultProps.sandbox.url);
expect(appHostUtils.setupSandboxProxyIframe).toHaveBeenCalledWith(
defaultProps.sandbox.url,
expect.any(Object),
);
});
});

Expand Down Expand Up @@ -279,6 +282,39 @@ describe('<AppFrame />', () => {
});

describe('lifecycle', () => {
it('should cancel sandbox timeout when effect cleanup fires before await resolves (StrictMode)', async () => {
// Simulate the cancelRef being populated synchronously (as the real impl does).
// Capture the cancelRef passed to setupSandboxProxyIframe and verify cancel() is
// called before the mock ever resolves — this is the Strict Mode timing scenario.
let capturedCancelRef: { cancel?: () => void } | undefined;
const cancelSpy = vi.fn();

vi.mocked(appHostUtils.setupSandboxProxyIframe).mockImplementation(
async (_url, cancelRef) => {
capturedCancelRef = cancelRef;
// Synchronously populate the cancelRef, mirroring the real implementation
if (cancelRef) {
cancelRef.cancel = cancelSpy;
}
// Return a promise that never resolves, simulating a slow sandbox
return new Promise(() => {});
},
);

const { unmount } = render(<AppFrame {...getPropsWithBridge()} />);

// Flush microtasks so the mock implementation runs
await act(async () => {});

expect(capturedCancelRef).toBeDefined();

// Unmount (simulates Strict Mode cleanup between effect invocations)
unmount();

// cancel() must have been called to clear the orphaned timeout
expect(cancelSpy).toHaveBeenCalledTimes(1);
});

it('should preserve iframe across re-renders', async () => {
const { rerender } = render(<AppFrame {...getPropsWithBridge()} />);

Expand Down Expand Up @@ -329,7 +365,10 @@ describe('<AppFrame />', () => {
// Should call setupSandboxProxyIframe again with new URL
await waitFor(() => {
expect(appHostUtils.setupSandboxProxyIframe).toHaveBeenCalledTimes(2);
expect(appHostUtils.setupSandboxProxyIframe).toHaveBeenLastCalledWith(newSandboxUrl);
expect(appHostUtils.setupSandboxProxyIframe).toHaveBeenLastCalledWith(
newSandboxUrl,
expect.any(Object),
);
});
});

Expand Down
65 changes: 65 additions & 0 deletions sdks/typescript/client/src/utils/app-host-utils.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';

import { SANDBOX_PROXY_READY_METHOD } from '@modelcontextprotocol/ext-apps/app-bridge';

import { setupSandboxProxyIframe, type SandboxCancelRef } from './app-host-utils';

describe('setupSandboxProxyIframe', () => {
beforeEach(() => {
vi.useFakeTimers();
});

afterEach(() => {
vi.useRealTimers();
});

it('rejects onReady when sandbox never becomes ready (timeout)', async () => {
const cancelRef: SandboxCancelRef = {};
const { onReady } = await setupSandboxProxyIframe(
new URL('https://example.com/proxy'),
cancelRef,
);

const assertion = expect(onReady).rejects.toThrow(
/Timed out waiting for sandbox proxy iframe to be ready/,
);
await vi.advanceTimersByTimeAsync(10_000);
await assertion;
});

it('resolves onReady when cancel is called before the sandbox handshake completes', async () => {
const cancelRef: SandboxCancelRef = {};
const { onReady } = await setupSandboxProxyIframe(
new URL('https://example.com/proxy'),
cancelRef,
);

expect(cancelRef.cancel).toBeTypeOf('function');
cancelRef.cancel!();

await expect(onReady).resolves.toBeUndefined();

await vi.advanceTimersByTimeAsync(10_000);
});

it('resolves onReady when the sandbox posts the ready message', async () => {
const { iframe, onReady } = await setupSandboxProxyIframe(new URL('https://example.com/proxy'));

// jsdom does not attach contentWindow to programmatic iframes; the handler matches
// event.source === iframe.contentWindow, so expose a stable mock window reference.
const sandboxWindow = {} as Window;
Object.defineProperty(iframe, 'contentWindow', {
get: () => sandboxWindow,
configurable: true,
});

window.dispatchEvent(
new MessageEvent('message', {
source: sandboxWindow,
data: { method: SANDBOX_PROXY_READY_METHOD },
}),
);

await expect(onReady).resolves.toBeUndefined();
});
});
25 changes: 24 additions & 1 deletion sdks/typescript/client/src/utils/app-host-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,19 @@ import { type Client } from '@modelcontextprotocol/sdk/client/index.js';
import { type Tool } from '@modelcontextprotocol/sdk/types.js';
const DEFAULT_SANDBOX_TIMEOUT_MS = 10000;

export async function setupSandboxProxyIframe(sandboxProxyUrl: URL): Promise<{
/**
* Assigned synchronously inside the onReady executor so callers can invoke
* `cancel()` while still awaiting `setupSandboxProxyIframe` or `onReady`.
* Cancelling clears timers and listeners and resolves `onReady`.
*/
export interface SandboxCancelRef {
cancel?: () => void;
}

export async function setupSandboxProxyIframe(
sandboxProxyUrl: URL,
cancelRef?: SandboxCancelRef,
): Promise<{
iframe: HTMLIFrameElement;
onReady: Promise<void>;
}> {
Expand All @@ -34,6 +46,17 @@ export async function setupSandboxProxyIframe(sandboxProxyUrl: URL): Promise<{
}
}, DEFAULT_SANDBOX_TIMEOUT_MS);

if (cancelRef) {
cancelRef.cancel = () => {
if (!settled) {
settled = true;
clearTimeout(timeoutId);
cleanup();
resolve();
}
};
}

const messageListener = (event: MessageEvent) => {
if (event.source === iframe.contentWindow) {
if (event.data && event.data.method === SANDBOX_PROXY_READY_METHOD) {
Expand Down
Loading