Skip to content

Commit fec5426

Browse files
chrstnbgemini-code-assist[bot]
authored andcommitted
Remove MCP servers on extension uninstall (#18121)
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
1 parent 6039af8 commit fec5426

2 files changed

Lines changed: 69 additions & 4 deletions

File tree

packages/core/src/tools/mcp-client-manager.test.ts

Lines changed: 54 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import {
1616
import { McpClientManager } from './mcp-client-manager.js';
1717
import { McpClient, MCPDiscoveryState } from './mcp-client.js';
1818
import type { ToolRegistry } from './tool-registry.js';
19-
import type { Config } from '../config/config.js';
19+
import type { Config, GeminiCLIExtension } from '../config/config.js';
2020

2121
vi.mock('./mcp-client.js', async () => {
2222
const originalModule = await vi.importActual('./mcp-client.js');
@@ -320,4 +320,57 @@ describe('McpClientManager', () => {
320320
await expect(manager.restartServer('test-server')).resolves.not.toThrow();
321321
});
322322
});
323+
324+
describe('Extension handling', () => {
325+
it('should remove mcp servers from allServerConfigs when stopExtension is called', async () => {
326+
const manager = new McpClientManager('0.0.1', toolRegistry, mockConfig);
327+
const mcpServers = {
328+
'test-server': { command: 'node', args: ['server.js'] },
329+
};
330+
const extension: GeminiCLIExtension = {
331+
name: 'test-extension',
332+
mcpServers,
333+
isActive: true,
334+
version: '1.0.0',
335+
path: '/some-path',
336+
contextFiles: [],
337+
id: '123',
338+
};
339+
340+
await manager.startExtension(extension);
341+
expect(manager.getMcpServers()).toHaveProperty('test-server');
342+
343+
await manager.stopExtension(extension);
344+
expect(manager.getMcpServers()).not.toHaveProperty('test-server');
345+
});
346+
347+
it('should remove servers from blockedMcpServers when stopExtension is called', async () => {
348+
mockConfig.getBlockedMcpServers.mockReturnValue(['blocked-server']);
349+
const manager = new McpClientManager('0.0.1', toolRegistry, mockConfig);
350+
const mcpServers = {
351+
'blocked-server': { command: 'node', args: ['server.js'] },
352+
};
353+
const extension: GeminiCLIExtension = {
354+
name: 'test-extension',
355+
mcpServers,
356+
isActive: true,
357+
version: '1.0.0',
358+
path: '/some-path',
359+
contextFiles: [],
360+
id: '123',
361+
};
362+
363+
await manager.startExtension(extension);
364+
expect(manager.getBlockedMcpServers()).toContainEqual({
365+
name: 'blocked-server',
366+
extensionName: 'test-extension',
367+
});
368+
369+
await manager.stopExtension(extension);
370+
expect(manager.getBlockedMcpServers()).not.toContainEqual({
371+
name: 'blocked-server',
372+
extensionName: 'test-extension',
373+
});
374+
});
375+
});
323376
});

packages/core/src/tools/mcp-client-manager.ts

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -72,9 +72,21 @@ export class McpClientManager {
7272
async stopExtension(extension: GeminiCLIExtension) {
7373
debugLogger.log(`Unloading extension: ${extension.name}`);
7474
await Promise.all(
75-
Object.keys(extension.mcpServers ?? {}).map((name) =>
76-
this.disconnectClient(name, true),
77-
),
75+
Object.keys(extension.mcpServers ?? {}).map((name) => {
76+
const config = this.allServerConfigs.get(name);
77+
if (config?.extension === extension) {
78+
this.allServerConfigs.delete(name);
79+
// Also remove from blocked servers if present
80+
const index = this.blockedMcpServers.findIndex(
81+
(s) => s.name === name && s.extensionName === extension.name,
82+
);
83+
if (index !== -1) {
84+
this.blockedMcpServers.splice(index, 1);
85+
}
86+
return this.disconnectClient(name, true);
87+
}
88+
return Promise.resolve();
89+
}),
7890
);
7991
await this.cliConfig.refreshMcpContext();
8092
}

0 commit comments

Comments
 (0)