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
17 changes: 11 additions & 6 deletions api/server/controllers/mcp.js
Original file line number Diff line number Diff line change
Expand Up @@ -219,13 +219,17 @@ const getMCPTools = async (req, res) => {

let serverTools;
let publicationGeneration;
let publicationRevision;
try {
({ tools: serverTools, publicationGeneration } =
await mcpManager.getServerToolFunctionsSnapshot(
userId,
serverName,
mcpConfig[serverName],
));
({
tools: serverTools,
publicationGeneration,
publicationRevision,
} = await mcpManager.getServerToolFunctionsSnapshot(
userId,
serverName,
mcpConfig[serverName],
));
} catch (error) {
logger.error(`[getMCPTools] Error fetching tools for server ${serverName}:`, error);
continue;
Expand All @@ -243,6 +247,7 @@ const getMCPTools = async (req, res) => {
serverTools,
serverConfig: mcpConfig[serverName],
publicationGeneration,
publicationRevision,
}).catch((err) =>
logger.error(`[getMCPTools] Failed to cache tools for ${serverName}:`, err),
);
Expand Down
16 changes: 16 additions & 0 deletions api/server/services/Tools/mcp.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ async function reinitMCPServer({
let oauthExpiresAt;
let ephemeralServer = false;
let publicationGeneration;
let publicationRevision;

try {
const registry = getMCPServersRegistry();
Expand Down Expand Up @@ -279,6 +280,20 @@ async function reinitMCPServer({
}
if (snapshot.complete) {
tools = snapshot.tools;
/** Reserved before this snapshot's tools/list; an app-level catalog cannot publish
* without it, and allocating a later one here would outrank fresher tools. */
publicationRevision = snapshot.publicationRevision;
if (snapshot.orderingUnavailable && typeof connection.refreshToolList === 'function') {
/** These tools still serve this request; the connection republishes the shared
* catalog under backoff rather than leaving it cold until the next reinitialize. */
connection
.refreshToolList()
.catch((err) =>
logger.debug(
`[MCP Reinitialize] Could not schedule a catalog republish for ${serverName}: ${err?.message ?? String(err)}`,
),
);
}
} else {
logger.warn(
`[MCP Reinitialize] Preserving cached tools for ${serverName} because tools/list returned an incomplete snapshot`,
Expand Down Expand Up @@ -306,6 +321,7 @@ async function reinitMCPServer({
tools,
serverConfig,
...(publicationGeneration && { publicationGeneration }),
...(publicationRevision && { publicationRevision }),
});
if (availableTools == null) {
tools = null;
Expand Down
44 changes: 44 additions & 0 deletions api/server/services/Tools/mcp.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,50 @@ describe('reinitMCPServer β€” customUserVars gating (issue #10969)', () => {
});
});

/** An app-level catalog write is dropped unless it carries the ordering reserved before its
* own tools/list. When this path forwarded no revision, every publication was discarded and
* agents were told the server had no tools at all (#14857). */
it('publishes under the ordering its snapshot was fetched with', async () => {
mockGetConnection.mockResolvedValue({
fetchOrderedToolsSnapshot: jest.fn().mockResolvedValue({
tools: [{ name: 'search', inputSchema: { type: 'object' } }],
complete: true,
publicationRevision: '7',
}),
});

await reinitMCPServer({
user,
serverName,
serverConfig: { type: 'streamable-http', url: 'https://thingy.example.com/mcp' },
});

expect(mockUpdateMCPServerTools).toHaveBeenCalledWith(
expect.objectContaining({ serverName, publicationRevision: '7' }),
);
});

it('asks the connection to republish a catalog it could not order', async () => {
const refreshToolList = jest.fn().mockResolvedValue(undefined);
mockGetConnection.mockResolvedValue({
refreshToolList,
fetchOrderedToolsSnapshot: jest.fn().mockResolvedValue({
tools: [{ name: 'search', inputSchema: { type: 'object' } }],
complete: true,
orderingUnavailable: true,
}),
});

const result = await reinitMCPServer({
user,
serverName,
serverConfig: { type: 'streamable-http', url: 'https://thingy.example.com/mcp' },
});

expect(refreshToolList).toHaveBeenCalledTimes(1);
expect(result.tools).toHaveLength(1);
});

it('preserves cached tools when live recovery returns an incomplete snapshot', async () => {
const fetchOrderedToolsSnapshot = jest.fn().mockResolvedValue({
tools: [{ name: 'partial', inputSchema: { type: 'object' } }],
Expand Down
15 changes: 14 additions & 1 deletion packages/api/src/mcp/ConnectionsRepository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -162,18 +162,30 @@ export class ConnectionsRepository {

this.connections.set(serverName, connection);
if (this.ownerId === undefined && options.refreshTools !== false) {
/** The snapshot carries ordering reserved before its own `tools/list`, so this
* first-connect publication is ordered against concurrent replicas exactly as a
* list_changed refresh is. An app-level write that cannot be ordered is dropped, which
* left agents with a permanently empty catalog when this path populated it (#14857). */
if (connection.client.getServerCapabilities()?.tools == null) {
const ordering = await connection.reserveToolsPublicationRevision();
/** The refresh path reserves again under backoff. Publishing unordered instead would be
* dropped in silence, leaving whatever this server last advertised in place. */
if (ordering.orderingUnavailable) {
await connection.refreshToolList();
return connection;
}
await notifyMCPToolsChanged({
tools: [],
serverName,
serverConfig,
publicationGeneration,
publicationRevision: ordering.publicationRevision,
});
return connection;
}
const initialGeneration = toolsChangedGeneration;
const snapshot = await connection.fetchToolsSnapshot();
if (snapshot.complete) {
if (snapshot.complete && !snapshot.orderingUnavailable) {
if (toolsChangedGeneration !== initialGeneration) {
await latestToolsChangedPublication;
} else {
Expand All @@ -182,6 +194,7 @@ export class ConnectionsRepository {
serverName,
serverConfig,
publicationGeneration,
publicationRevision: snapshot.publicationRevision,
});
}
} else {
Expand Down
7 changes: 3 additions & 4 deletions packages/api/src/mcp/MCPManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -422,6 +422,7 @@ export class MCPManager extends UserConnectionManager {
): Promise<{
tools: t.LCAvailableTools | null;
publicationGeneration?: string;
publicationRevision?: string;
}> {
try {
const registry = MCPServersRegistry.getInstance();
Expand All @@ -434,9 +435,7 @@ export class MCPManager extends UserConnectionManager {
? await this.appConnections?.get(serverName)
: null;
if (existingAppConnection != null) {
return {
tools: await MCPServerInspector.getToolFunctions(serverName, existingAppConnection),
};
return MCPServerInspector.getToolCatalog(serverName, existingAppConnection);
}

let awaitedRecovery: Promise<void> | undefined;
Expand Down Expand Up @@ -482,7 +481,7 @@ export class MCPManager extends UserConnectionManager {
}

try {
const tools = await MCPServerInspector.getToolFunctions(serverName, connection);
const { tools } = await MCPServerInspector.getToolCatalog(serverName, connection);
const generationAfterFetch = await getMCPToolsChangedGeneration({ userId, serverName });
if (
publicationGeneration != null &&
Expand Down
73 changes: 72 additions & 1 deletion packages/api/src/mcp/__tests__/ConnectionsRepository.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
import { logger } from '@librechat/data-schemas';
import type * as t from '~/mcp/types';
import { getMCPAppToolsPublicationGeneration, setMCPToolsChangedHandler } from '~/mcp/toolsChanged';
import {
setMCPToolsChangedHandler,
setMCPToolsChangedRevisionHandler,
getMCPAppToolsPublicationGeneration,
} from '~/mcp/toolsChanged';
import { ConnectionsRepository } from '~/mcp/ConnectionsRepository';
import { MCPConnectionFactory } from '~/mcp/MCPConnectionFactory';
import { MCPConnection } from '~/mcp/connection';
Expand Down Expand Up @@ -83,6 +87,7 @@ describe('ConnectionsRepository', () => {
disconnect: jest.fn().mockResolvedValue(undefined),
dispose: jest.fn().mockResolvedValue(undefined),
fetchToolsSnapshot: jest.fn().mockResolvedValue({ tools: [], complete: true }),
reserveToolsPublicationRevision: jest.fn().mockResolvedValue({}),
refreshToolList: jest.fn().mockResolvedValue(undefined),
createdAt: Date.now(),
isStale: jest.fn().mockReturnValue(false),
Expand All @@ -101,6 +106,7 @@ describe('ConnectionsRepository', () => {

afterEach(() => {
setMCPToolsChangedHandler(null);
setMCPToolsChangedRevisionHandler(null);
jest.clearAllMocks();
});

Expand Down Expand Up @@ -185,6 +191,71 @@ describe('ConnectionsRepository', () => {
await expect(load).resolves.toBe(mockConnection);
});

/** An app-level write that carries no revision cannot be ordered and is dropped, so the
* ordering reserved for this snapshot has to reach the publication (#14857). */
it('publishes the initial app snapshot under the revision it was fetched with', async () => {
mockConnection.fetchToolsSnapshot.mockResolvedValue({
tools: [],
complete: true,
publicationRevision: '4',
});
const handler = jest.fn();
setMCPToolsChangedHandler(handler);

await repository.get('server1');

expect(handler).toHaveBeenCalledWith(
expect.objectContaining({ serverName: 'server1', publicationRevision: '4' }),
);
});

it('reserves ordering for a server that advertises no tools capability', async () => {
(mockConnection.client.getServerCapabilities as jest.Mock).mockReturnValue({});
mockConnection.reserveToolsPublicationRevision = jest
.fn()
.mockResolvedValue({ publicationRevision: '9' });
const handler = jest.fn();
setMCPToolsChangedHandler(handler);

await repository.get('server1');

expect(mockConnection.fetchToolsSnapshot).not.toHaveBeenCalled();
expect(handler).toHaveBeenCalledWith(
expect.objectContaining({ serverName: 'server1', tools: [], publicationRevision: '9' }),
);
});

/** An unordered publication is dropped in silence, so a server left with no way to order
* its empty catalog has to retry rather than leave the previous one advertised. */
it('retries an empty catalog it could not reserve ordering for', async () => {
(mockConnection.client.getServerCapabilities as jest.Mock).mockReturnValue({});
mockConnection.reserveToolsPublicationRevision = jest
.fn()
.mockResolvedValue({ orderingUnavailable: true });
const handler = jest.fn();
setMCPToolsChangedHandler(handler);

await repository.get('server1');

expect(mockConnection.refreshToolList).toHaveBeenCalledTimes(1);
expect(handler).not.toHaveBeenCalled();
});

it('retries a fetched catalog it could not reserve ordering for', async () => {
mockConnection.fetchToolsSnapshot.mockResolvedValue({
tools: [],
complete: true,
orderingUnavailable: true,
});
const handler = jest.fn();
setMCPToolsChangedHandler(handler);

await repository.get('server1');

expect(mockConnection.refreshToolList).toHaveBeenCalledTimes(1);
expect(handler).not.toHaveBeenCalled();
});

it('can defer the initial app tool refresh for startup synchronization', async () => {
await repository.get('server1', { refreshTools: false });

Expand Down
Loading
Loading