fix(mcp): address Codex round-3 findings declined in error

Re-verification against source showed five previously declined Codex threads
had merit, so this implements the fixes rather than the incorrect push-backs.

Restores rendering of inline ui:// text resources attached to a tool call.
The selector in ToolCall excluded text-bearing resources and ToolCallInfo no
longer received attachments, so an inline resource vanished unless the model
echoed a marker. MCPAppView now renders every renderable attachment and routes
text resources through its srcDoc branch.

Wires metadata cache invalidation. clearResourceUriCache had no caller, so the
resourceUri, modelOnly and knownToolNames caches were never cleared. Tearing
down a user connection now evicts that server/user cache entry.

Threads the original registry config through the iframe follow-up RPCs.
readResource and appToolCall share a getAppConnection helper that resolves the
server config, hands it to getConnection so config-source servers resolve,
refreshes headers for non-DB-sourced servers, and rejects configs that still
need request body placeholders the app context cannot supply.

Completes the host side of the App Bridge so migrated mcp-ui apps keep working:
advertises serverResources and sets onreadresource for lazy resource loads, and
wires onmessage so ui/message (the standardized replacement for mcp-ui prompt
and intent actions) reaches the conversation.

Honors a dedicated sandbox origin. getMCPSandboxUrl passes the parent origin
to the sandbox, which the proxy reads from the parentOrigin query param so the
handshake targets LibreChat instead of the sandbox's own origin.
This commit is contained in:
Dustin Healy 2026-06-23 21:16:34 -07:00
parent 1f9d038291
commit ae0e98e4c9
8 changed files with 180 additions and 44 deletions

View file

@ -18,9 +18,18 @@
let readyInterval = null;
const SANDBOX_PREFIX = 'ui/notifications/sandbox-';
// The sandbox is always served same-origin with LibreChat, so window.location.origin
// is the exact expected parent origin. No referrer fallback or lazy-set needed.
const trustedOrigin = window.location.origin;
// Default to same-origin. When the sandbox is served from a dedicated origin, the parent
// passes its origin via the parentOrigin query param so the handshake targets LibreChat
// rather than the sandbox's own origin.
const trustedOrigin = (function () {
try {
const param = new URLSearchParams(window.location.search).get('parentOrigin');
if (param) {
return new URL(param).origin;
}
} catch (e) {}
return window.location.origin;
})();
function notifyReady() {
window.parent.postMessage(

View file

@ -267,12 +267,15 @@ export default function ToolCall({
[args, output],
);
const mcpApp = useMemo(() => {
const mcpApps = useMemo(() => {
const uiResources: UIResource[] =
attachments
?.filter((a) => a.type === Tools.ui_resources)
.flatMap((a) => (a[Tools.ui_resources] ?? []) as UIResource[]) ?? [];
return uiResources.find((r) => r.toolName && r.serverName && !r.text) ?? null;
return uiResources.filter(
(r) =>
(r.toolName && r.serverName) || (r.text && (r.mimeType ?? 'text/html').includes('html')),
);
}, [attachments]);
const authDomain = useMemo(() => {
@ -388,7 +391,8 @@ export default function ToolCall({
{!hideAttachments && attachments && attachments.length > 0 && (
<AttachmentGroup attachments={attachments} />
)}
{mcpApp && hasOutput && <MCPAppView key={mcpApp.resourceId} app={mcpApp} args={_args} />}
{hasOutput &&
mcpApps.map((app) => <MCPAppView key={app.resourceId} app={app} args={_args} />)}
</>
);
}

View file

@ -35,6 +35,7 @@ jest.mock('~/hooks', () => ({
jest.mock('~/hooks/MCP', () => ({
useMCPIconMap: () => new Map(),
useAppBridge: jest.fn(),
}));
jest.mock('~/components/Chat/Messages/Content/MessageContent', () => ({
@ -175,6 +176,35 @@ describe('ToolCall', () => {
const attachmentGroup = screen.getByTestId('attachment-group');
expect(JSON.parse(attachmentGroup.textContent!)).toEqual(attachments);
});
it('renders an iframe for an inline ui:// text resource attached to the tool call', () => {
const attachments = [
{
type: Tools.ui_resources,
messageId: 'msg1',
toolCallId: 'tool1',
conversationId: 'conv1',
[Tools.ui_resources]: [
{
uri: 'ui://test-server/inline.html',
mimeType: 'text/html',
text: '<p>inline resource</p>',
resourceId: 'inline-1',
toolName: 'test-tool',
serverName: 'test-server',
},
],
},
];
const { container } = renderWithRecoil(
<ToolCall {...mockProps} attachments={attachments as any} />,
);
const iframe = container.querySelector('iframe[srcdoc]');
expect(iframe).toBeInTheDocument();
expect(iframe).toHaveAttribute('srcdoc', '<p>inline resource</p>');
});
});
describe('attachment group rendering', () => {

View file

@ -23,7 +23,6 @@ Object.defineProperty(HTMLElement.prototype, 'scrollTo', {
const makeResource = (n: number): UIResource => ({
uri: `resource${n}`,
mimeType: 'text/html',
text: `Resource ${n}`,
resourceId: `r${n}`,
toolName: 'test-tool',
serverName: 'test-server',

View file

@ -6,10 +6,13 @@ import {
buildAllowAttribute,
} from '@modelcontextprotocol/ext-apps/app-bridge';
import type { UIResource } from 'librechat-data-provider';
import { callMCPAppTool, fetchMCPResourceHtml } from '~/utils/mcpApps';
import { callMCPAppTool, fetchMCPResourceHtml, readMCPResource } from '~/utils/mcpApps';
import { useOptionalMessagesOperations } from '~/Providers';
import { logger } from '~/utils';
import store from '~/store';
type MessageContentBlock = { type?: string; text?: string };
type SizeParams = { width?: number; height?: number };
export function useAppBridge(
@ -20,7 +23,12 @@ export function useAppBridge(
onSizeChanged: (params: SizeParams) => void,
) {
const user = useRecoilValue(store.user);
const { ask } = useOptionalMessagesOperations();
const bridgeRef = useRef<AppBridge | null>(null);
const askRef = useRef(ask);
useEffect(() => {
askRef.current = ask;
});
// Refs keep latest values accessible inside the stable effect closure without triggering remount.
// The bridge mounts once per resourceId; toolArgs/toolResult/onSizeChanged are captured at
@ -56,7 +64,7 @@ export function useAppBridge(
bridge = new AppBridge(
null,
{ name: 'LibreChat', version: '1.0.0' },
{ openLinks: {}, serverTools: {}, logging: {} },
{ openLinks: {}, serverTools: {}, serverResources: {}, logging: {} },
{
hostContext: {
theme,
@ -81,6 +89,20 @@ export function useAppBridge(
return {};
};
bridge.onreadresource = async (params) =>
readMCPResource(resource.serverName as string, params.uri, user?.id) as never;
bridge.onmessage = async ({ content }) => {
const text = (content as MessageContentBlock[])
.filter((block) => block.type === 'text' && typeof block.text === 'string')
.map((block) => block.text)
.join('\n');
if (text) {
askRef.current({ text });
}
return {};
};
bridge.addEventListener('sandboxready', async () => {
try {
const { html, csp, permissions } = await fetchMCPResourceHtml(

View file

@ -2,8 +2,14 @@ import { request, apiBaseUrl } from 'librechat-data-provider';
export function getMCPSandboxUrl(): string {
const configured = (import.meta.env as Record<string, string | undefined>).VITE_MCP_SANDBOX_URL;
if (configured) return configured;
return `${window.location.origin}${apiBaseUrl()}/api/mcp/sandbox`;
const base = configured ?? `${window.location.origin}${apiBaseUrl()}/api/mcp/sandbox`;
try {
const url = new URL(base, window.location.origin);
url.searchParams.set('parentOrigin', window.location.origin);
return url.toString();
} catch {
return base;
}
}
export async function callMCPAppTool(

View file

@ -347,7 +347,14 @@ ${formattedInstructions}
Please follow these instructions when using tools from the respective MCP servers.`;
}
public clearResourceUriCache(serverName?: string): void {
public clearResourceUriCache(serverName?: string, userId?: string): void {
if (serverName && userId != null) {
const cacheKey = `${serverName}:${userId}`;
this.resourceUriCache.delete(cacheKey);
this.modelOnlyToolCache.delete(cacheKey);
this.knownToolNamesCache.delete(cacheKey);
return;
}
if (serverName) {
for (const key of this.resourceUriCache.keys()) {
if (key === serverName || key.startsWith(`${serverName}:`)) {
@ -363,6 +370,11 @@ Please follow these instructions when using tools from the respective MCP server
}
}
protected removeUserConnection(userId: string, serverName: string): void {
this.clearResourceUriCache(serverName, userId);
super.removeUserConnection(userId, serverName);
}
private async populateToolCaches(connection: MCPConnection, cacheKey: string): Promise<void> {
const tools = await connection.fetchTools();
const serverMap = new Map<
@ -652,6 +664,66 @@ Please follow these instructions when using tools from the respective MCP server
* Reads a UI resource from an MCP server.
* Used by MCP Apps iframes to fetch additional resources via the host.
*/
/**
* Resolves the same registry-backed config the original tool call used and hands it to
* getConnection so config-source servers resolve, then refreshes headers for non-DB-sourced
* servers. Iframe follow-up requests arrive without the original requestBody, so configs that
* still need runtime body placeholders are rejected rather than connected with unresolved values.
*/
private async getAppConnection({
serverName,
userId,
user,
}: {
serverName: string;
userId: string;
user?: IUser;
}): Promise<MCPConnection> {
const logPrefix = `[MCP][User: ${userId}][${serverName}]`;
const rawConfig = await MCPServersRegistry.getInstance().getServerConfig(serverName, userId);
const isDbSourced = rawConfig ? isUserSourced(rawConfig) : false;
if (rawConfig) {
if (rawConfig.obo) {
throw new McpError(
ErrorCode.InvalidRequest,
`${logPrefix} Server "${serverName}" requires per-call OBO token resolution which is not supported for app requests.`,
);
}
if (!isDbSourced && mcpOptionsContainGraphTokenPlaceholder(rawConfig as t.MCPOptions)) {
throw new McpError(
ErrorCode.InvalidRequest,
`${logPrefix} Server "${serverName}" requires Graph API token resolution which is not supported for app requests.`,
);
}
const missingBodyFields = getMissingRuntimeBodyPlaceholderFields(rawConfig);
if (missingBodyFields.length > 0) {
throw new McpError(
ErrorCode.InvalidRequest,
`${logPrefix} Server "${serverName}" requires request body field(s) (${missingBodyFields.join(', ')}) that are not available for app requests.`,
);
}
}
const connection = await this.getConnection({
serverName,
user,
serverConfig: rawConfig ?? undefined,
});
if (rawConfig && !isDbSourced) {
const currentOptions = processMCPEnv({
user,
dbSourced: false,
options: rawConfig as t.MCPOptions,
});
const resolvedHeaders: Record<string, string> =
'headers' in currentOptions ? { ...(currentOptions.headers || {}) } : {};
connection.setRequestHeaders(resolvedHeaders);
}
return connection;
}
async readResource({
userId,
serverName,
@ -665,7 +737,7 @@ Please follow these instructions when using tools from the respective MCP server
}): Promise<unknown> {
const logPrefix = `[MCP][User: ${userId}][${serverName}]`;
if (userId && user) this.updateUserLastActivity(userId);
const connection = await this.getConnection({ serverName, user });
const connection = await this.getAppConnection({ serverName, userId, user });
if (!(await connection.isConnected())) {
throw new McpError(
@ -705,7 +777,7 @@ Please follow these instructions when using tools from the respective MCP server
}): Promise<unknown> {
const logPrefix = `[MCP][User: ${userId}][${serverName}]`;
if (userId && user) this.updateUserLastActivity(userId);
const connection = await this.getConnection({ serverName, user });
const connection = await this.getAppConnection({ serverName, userId, user });
if (!(await connection.isConnected())) {
throw new McpError(
@ -732,36 +804,6 @@ Please follow these instructions when using tools from the respective MCP server
);
}
const rawConfig = await MCPServersRegistry.getInstance().getServerConfig(serverName, userId);
if (rawConfig) {
if (rawConfig.obo) {
throw new McpError(
ErrorCode.InvalidRequest,
`${logPrefix} Server "${serverName}" requires per-call OBO token resolution which is not supported for app tool calls.`,
);
}
const isDbSourced = isUserSourced(rawConfig);
if (!isDbSourced && mcpOptionsContainGraphTokenPlaceholder(rawConfig as t.MCPOptions)) {
throw new McpError(
ErrorCode.InvalidRequest,
`${logPrefix} Server "${serverName}" requires Graph API token resolution which is not supported for app tool calls.`,
);
}
// DB-sourced servers have their customUserVars (e.g. {{MCP_API_KEY}}) resolved during
// the original callTool setup. Re-processing without customUserVars would overwrite
// those resolved headers with unresolved placeholders, so skip for DB-sourced servers.
if (!isDbSourced) {
const currentOptions = processMCPEnv({
user,
dbSourced: false,
options: rawConfig as t.MCPOptions,
});
const resolvedHeaders: Record<string, string> =
'headers' in currentOptions ? { ...(currentOptions.headers || {}) } : {};
connection.setRequestHeaders(resolvedHeaders);
}
}
const result = await connection.client.request(
{
method: 'tools/call',

View file

@ -1182,6 +1182,30 @@ describe('MCPManager', () => {
});
});
describe('appToolCall - app request context', () => {
const mockUser: Partial<IUser> = { id: 'user-123' };
it('rejects when the server config needs request body placeholders unavailable to app calls', async () => {
(mockRegistryInstance.getServerConfig as jest.Mock).mockResolvedValue({
source: 'yaml',
type: 'sse',
url: 'https://example.com/{{LIBRECHAT_BODY_CONVERSATIONID}}/mcp',
});
const manager = await MCPManager.createInstance(newMCPServersConfig());
await expect(
manager.appToolCall({
userId: 'user-123',
serverName: 'body-server',
toolName: 'do_thing',
toolArguments: {},
user: mockUser as IUser,
}),
).rejects.toThrow(/request body field/);
});
});
describe('getConnection', () => {
const mockUser: Partial<IUser> = {
id: 'user-123',