mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-08-28 04:37:37 +00:00
fix(mcp): handle template-only servers, render UI-only results, keep the declared app
Guard the resources/list request in getAdvertisedResources so a server that only implements resources/templates/list still collects templates and can authorize template-matching reads instead of denying them. Render MCP app views whenever a tool result carries a ui_resources attachment, not only when the tool also returned text output, so a UI-only result no longer silently loses its view. Synthesize the tool-declared ui:// app resource unless the result already returned that exact URI, so a secondary ui:// resource in the same result no longer suppresses the declared app.
This commit is contained in:
parent
8f10ef4b1f
commit
817e35561a
5 changed files with 95 additions and 17 deletions
|
|
@ -404,7 +404,7 @@ export default function ToolCall({
|
|||
{!hideAttachments && attachments && attachments.length > 0 && (
|
||||
<AttachmentGroup attachments={attachments} />
|
||||
)}
|
||||
{hasOutput &&
|
||||
{mcpApps.length > 0 &&
|
||||
mcpApps.map((app) => <MCPAppView key={app.resourceId} app={app} args={_args} />)}
|
||||
</>
|
||||
);
|
||||
|
|
|
|||
|
|
@ -977,19 +977,25 @@ Please follow these instructions when using tools from the respective MCP server
|
|||
|
||||
const uris = new Set<string>();
|
||||
let cursor: string | undefined;
|
||||
for (let page = 0; page < MCPManager.RESOURCE_LIST_MAX_PAGES; page++) {
|
||||
const result: ListResourcesResult = await connection.client.request(
|
||||
{ method: 'resources/list', params: cursor != null ? { cursor } : {} },
|
||||
ListResourcesResultSchema,
|
||||
{ timeout: connection.timeout },
|
||||
);
|
||||
for (const resource of result.resources) {
|
||||
uris.add(resource.uri);
|
||||
// A template-only server may not implement resources/list; treat its failure as an empty
|
||||
// concrete list so advertised templates below are still collected and can authorize reads.
|
||||
try {
|
||||
for (let page = 0; page < MCPManager.RESOURCE_LIST_MAX_PAGES; page++) {
|
||||
const result: ListResourcesResult = await connection.client.request(
|
||||
{ method: 'resources/list', params: cursor != null ? { cursor } : {} },
|
||||
ListResourcesResultSchema,
|
||||
{ timeout: connection.timeout },
|
||||
);
|
||||
for (const resource of result.resources) {
|
||||
uris.add(resource.uri);
|
||||
}
|
||||
if (result.nextCursor == null) {
|
||||
break;
|
||||
}
|
||||
cursor = result.nextCursor;
|
||||
}
|
||||
if (result.nextCursor == null) {
|
||||
break;
|
||||
}
|
||||
cursor = result.nextCursor;
|
||||
} catch (error) {
|
||||
logger.debug(`[MCP][${cacheKey}] resources/list unavailable; using templates only.`, error);
|
||||
}
|
||||
|
||||
const templates: RegExp[] = [];
|
||||
|
|
|
|||
|
|
@ -1561,6 +1561,30 @@ describe('MCPManager', () => {
|
|||
const methods = request.mock.calls.map((c) => (c[0] as { method: string }).method);
|
||||
expect(methods).toContain('resources/read');
|
||||
});
|
||||
|
||||
it('authorizes a template match when the server does not implement resources/list', async () => {
|
||||
const request = jest.fn().mockImplementation((req: { method: string }) => {
|
||||
if (req.method === 'resources/list') {
|
||||
return Promise.reject(new Error('Method not found'));
|
||||
}
|
||||
if (req.method === 'resources/templates/list') {
|
||||
return Promise.resolve({ resourceTemplates: [{ uriTemplate: 'db://items/{id}' }] });
|
||||
}
|
||||
return Promise.resolve({ contents: [] });
|
||||
});
|
||||
const manager = await MCPManager.createInstance(newMCPServersConfig());
|
||||
jest.spyOn(manager, 'getConnection').mockResolvedValue(buildConnection(request));
|
||||
|
||||
await manager.readResource({
|
||||
userId: 'user-123',
|
||||
serverName: 'srv',
|
||||
uri: 'db://items/42',
|
||||
user: mockUser as IUser,
|
||||
});
|
||||
|
||||
const methods = request.mock.calls.map((c) => (c[0] as { method: string }).method);
|
||||
expect(methods).toContain('resources/read');
|
||||
});
|
||||
});
|
||||
|
||||
describe('getConnection', () => {
|
||||
|
|
|
|||
|
|
@ -357,6 +357,51 @@ describe('formatToolContent', () => {
|
|||
expect(uiResourceArtifact?.resultMeta).toBeUndefined();
|
||||
});
|
||||
|
||||
it('still synthesizes the tool-declared app when the result returns a different ui:// resource', () => {
|
||||
const result: t.MCPToolCallResponse = {
|
||||
content: [
|
||||
{
|
||||
type: 'resource',
|
||||
resource: {
|
||||
uri: 'ui://chart',
|
||||
mimeType: 'text/html;profile=mcp-app',
|
||||
text: '<p>c</p>',
|
||||
},
|
||||
},
|
||||
],
|
||||
};
|
||||
|
||||
const [, artifacts] = formatToolContent(result, 'openai', {
|
||||
serverName: 'srv',
|
||||
toolName: 'do_thing',
|
||||
resourceUri: 'ui://app',
|
||||
});
|
||||
|
||||
const uris = (artifacts?.ui_resources?.data ?? []).map((r) => r.uri);
|
||||
expect(uris).toContain('ui://chart');
|
||||
expect(uris).toContain('ui://app');
|
||||
});
|
||||
|
||||
it('does not double-synthesize when the returned resource is the declared app', () => {
|
||||
const result: t.MCPToolCallResponse = {
|
||||
content: [
|
||||
{
|
||||
type: 'resource',
|
||||
resource: { uri: 'ui://app', mimeType: 'text/html;profile=mcp-app', text: '<p>a</p>' },
|
||||
},
|
||||
],
|
||||
};
|
||||
|
||||
const [, artifacts] = formatToolContent(result, 'openai', {
|
||||
serverName: 'srv',
|
||||
toolName: 'do_thing',
|
||||
resourceUri: 'ui://app',
|
||||
});
|
||||
|
||||
const uris = (artifacts?.ui_resources?.data ?? []).map((r) => r.uri);
|
||||
expect(uris).toEqual(['ui://app']);
|
||||
});
|
||||
|
||||
it('suppresses embedded ui:// resources when apps are disabled for the scope', () => {
|
||||
const result: t.MCPToolCallResponse = {
|
||||
content: [
|
||||
|
|
|
|||
|
|
@ -316,13 +316,16 @@ export function formatToolContent(
|
|||
}
|
||||
}
|
||||
|
||||
// MCP Apps: if the tool declares a ui:// resourceUri but didn't include a resource
|
||||
// content item, create a synthetic UIResource so the frontend renders AppRenderer.
|
||||
// MCP Apps: the tool-declared ui:// resourceUri is the app the host renders for the call, so
|
||||
// synthesize it unless the result already returned that exact resource. A secondary ui://
|
||||
// resource in the result must not suppress the declared app.
|
||||
const declaredAppAlreadyReturned =
|
||||
metadata?.resourceUri != null && uiResources.some((r) => r.uri === metadata.resourceUri);
|
||||
if (
|
||||
uiResources.length === 0 &&
|
||||
metadata?.resourceUri &&
|
||||
metadata.serverName &&
|
||||
metadata.toolName
|
||||
metadata.toolName &&
|
||||
!declaredAppAlreadyReturned
|
||||
) {
|
||||
const resourceId = deriveResourceId(
|
||||
metadata.resourceUri,
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue