Skip to content

fix: MCP client global variable causes tool duplication #76

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Adancurusul wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: MCP client global variable causes tool duplication #76

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
34 changes: 25 additions & 9 deletions packages/core/src/tools/mcp-client.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,8 @@ const mcpServerStatusesInternal: Map<string, MCPServerStatus> = new Map();
*/
let mcpDiscoveryState: MCPDiscoveryState = MCPDiscoveryState.NOT_STARTED;

let mcpClient: Client | null = null;
// Store all active MCP client connections
const mcpClients: Map<string, Client> = new Map();

/**
* Event listeners for MCP server status changes
Expand Down Expand Up @@ -213,11 +214,15 @@ async function connectAndDiscover(
return;
}

mcpClient = new Client({
// Create a new client instance for each MCP server
const mcpClient = new Client({
name: 'gemini-cli-mcp-client',
version: '0.0.1',
});

// Store the client for later cleanup
mcpClients.set(mcpServerName, mcpClient);

// patch Client.callTool to use request timeout as genai McpCallTool.callTool does not do it
// TODO: remove this hack once GenAI SDK does callTool with request options
if ('callTool' in mcpClient) {
Expand Down Expand Up @@ -255,15 +260,17 @@ async function connectAndDiscover(
errorString += `\nMake sure it is available in the sandbox`;
}
console.error(errorString);
// Update status to disconnected
// Update status to disconnected and cleanup
updateMCPServerStatus(mcpServerName, MCPServerStatus.DISCONNECTED);
mcpClients.delete(mcpServerName);
return;
}

mcpClient.onerror = (error) => {
console.error(`MCP ERROR (${mcpServerName}):`, error.toString());
// Update status to disconnected on error
// Update status to disconnected on error and cleanup
updateMCPServerStatus(mcpServerName, MCPServerStatus.DISCONNECTED);
mcpClients.delete(mcpServerName);
};

if (transport instanceof StdioClientTransport && transport.stderr) {
Expand Down Expand Up @@ -291,8 +298,9 @@ async function connectAndDiscover(
) {
await transport.close();
}
// Update status to disconnected
// Update status to disconnected and cleanup
updateMCPServerStatus(mcpServerName, MCPServerStatus.DISCONNECTED);
mcpClients.delete(mcpServerName);
return;
}

Expand Down Expand Up @@ -368,8 +376,9 @@ async function connectAndDiscover(
) {
await transport.close();
}
// Update status to disconnected
// Update status to disconnected and cleanup
updateMCPServerStatus(mcpServerName, MCPServerStatus.DISCONNECTED);
mcpClients.delete(mcpServerName);
}

// If no tools were registered from this MCP server, the following 'if' block
Expand All @@ -387,8 +396,9 @@ async function connectAndDiscover(
transport instanceof StreamableHTTPClientTransport
) {
await transport.close();
// Update status to disconnected
// Update status to disconnected and cleanup
updateMCPServerStatus(mcpServerName, MCPServerStatus.DISCONNECTED);
mcpClients.delete(mcpServerName);
}
}
}
Expand All @@ -397,7 +407,13 @@ async function connectAndDiscover(
* Close all MCP connections.
*/
export async function closeAllMCPConnections() {
if (mcpClient) {
await mcpClient.close();
// Close all stored MCP client connections
for (const [serverName, client] of mcpClients.entries()) {
try {
await client.close();
mcpClients.delete(serverName);
} catch (error) {
console.error(`Error closing MCP client for ${serverName}:`, error);
}
}
}