diff --git a/sdks/typescript/client/src/components/AppFrame.tsx b/sdks/typescript/client/src/components/AppFrame.tsx index 2b02183e..aa570b4c 100644 --- a/sdks/typescript/client/src/components/AppFrame.tsx +++ b/sdks/typescript/client/src/components/AppFrame.tsx @@ -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. @@ -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 { @@ -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; @@ -237,6 +242,7 @@ export const AppFrame = (props: AppFrameProps) => { return () => { mounted = false; + cancelRef.cancel?.(); }; }, [sandbox.url, sandbox.csp, appBridge]); diff --git a/sdks/typescript/client/src/components/__tests__/AppFrame.test.tsx b/sdks/typescript/client/src/components/__tests__/AppFrame.test.tsx index a2bc1837..d1f508cf 100644 --- a/sdks/typescript/client/src/components/__tests__/AppFrame.test.tsx +++ b/sdks/typescript/client/src/components/__tests__/AppFrame.test.tsx @@ -119,7 +119,10 @@ describe('', () => { render(); await waitFor(() => { - expect(appHostUtils.setupSandboxProxyIframe).toHaveBeenCalledWith(defaultProps.sandbox.url); + expect(appHostUtils.setupSandboxProxyIframe).toHaveBeenCalledWith( + defaultProps.sandbox.url, + expect.any(Object), + ); }); }); @@ -279,6 +282,39 @@ describe('', () => { }); 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(); + + // 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(); @@ -329,7 +365,10 @@ describe('', () => { // 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), + ); }); }); diff --git a/sdks/typescript/client/src/utils/app-host-utils.test.ts b/sdks/typescript/client/src/utils/app-host-utils.test.ts new file mode 100644 index 00000000..b67ddafe --- /dev/null +++ b/sdks/typescript/client/src/utils/app-host-utils.test.ts @@ -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(); + }); +}); diff --git a/sdks/typescript/client/src/utils/app-host-utils.ts b/sdks/typescript/client/src/utils/app-host-utils.ts index 764de1aa..3110ada0 100644 --- a/sdks/typescript/client/src/utils/app-host-utils.ts +++ b/sdks/typescript/client/src/utils/app-host-utils.ts @@ -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; }> { @@ -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) {