Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
28 changes: 27 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,20 @@ 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<{
/**
* Mutable ref object that is populated synchronously inside the Promise constructor
* of setupSandboxProxyIframe, before the first async yield. This allows callers to
* cancel the pending timeout even if cleanup fires before the awaited result resolves
* (e.g. React Strict Mode double-invocation).
*/
export interface SandboxCancelRef {
cancel?: () => void;
}

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

// Populated synchronously (before any async yield) so the cleanup function
// returned from useEffect can always call cancelRef.cancel() — even when React
// Strict Mode fires cleanup before the awaited setupSandboxProxyIframe resolves.
if (cancelRef) {
cancelRef.cancel = () => {
if (!settled) {
settled = true;
clearTimeout(timeoutId);
cleanup();
Comment thread
matsjfunke marked this conversation as resolved.
Outdated
}
};
}

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