fix(core): prevent state corruption in McpClientManager during collis (#19782)

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

@@ -381,6 +381,36 @@ describe('McpClientManager', () => {
       expect(manager.getMcpServers()).not.toHaveProperty('test-server');
     });
 
+    it('should ignore an extension attempting to register a server with an existing name', async () => {
+      const manager = new McpClientManager('0.0.1', toolRegistry, mockConfig);
+      const userConfig = { command: 'node', args: ['user-server.js'] };
+
+      mockConfig.getMcpServers.mockReturnValue({
+        'test-server': userConfig,
+      });
+      mockedMcpClient.getServerConfig.mockReturnValue(userConfig);
+
+      await manager.startConfiguredMcpServers();
+      expect(mockedMcpClient.connect).toHaveBeenCalledTimes(1);
+
+      const extension: GeminiCLIExtension = {
+        name: 'test-extension',
+        mcpServers: {
+          'test-server': { command: 'node', args: ['ext-server.js'] },
+        },
+        isActive: true,
+        version: '1.0.0',
+        path: '/some-path',
+        contextFiles: [],
+        id: '123',
+      };
+
+      await manager.startExtension(extension);
+
+      expect(mockedMcpClient.disconnect).not.toHaveBeenCalled();
+      expect(mockedMcpClient.connect).toHaveBeenCalledTimes(1);
+    });
+
     it('should remove servers from blockedMcpServers when stopExtension is called', async () => {
       mockConfig.getBlockedMcpServers.mockReturnValue(['blocked-server']);
       const manager = new McpClientManager('0.0.1', toolRegistry, mockConfig);

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

@@ -176,6 +176,20 @@ export class McpClientManager {
     name: string,
     config: MCPServerConfig,
   ): Promise<void> {
+    const existing = this.clients.get(name);
+    if (
+      existing &&
+      existing.getServerConfig().extension?.id !== config.extension?.id
+    ) {
+      const extensionText = config.extension
+        ? ` from extension "${config.extension.name}"`
+        : '';
+      debugLogger.warn(
+        `Skipping MCP config for server with name "${name}"${extensionText} as it already exists.`,
+      );
+      return;
+    }
+
     // Always track server config for UI display
     this.allServerConfigs.set(name, config);
 
@@ -191,7 +205,6 @@ export class McpClientManager {
     }
     // User-disabled servers: disconnect if running, don't start
     if (await this.isDisabledByUser(name)) {
-      const existing = this.clients.get(name);
       if (existing) {
         await this.disconnectClient(name);
       }
@@ -203,16 +216,6 @@ export class McpClientManager {
     if (config.extension && !config.extension.isActive) {
       return;
     }
-    const existing = this.clients.get(name);
-    if (existing && existing.getServerConfig().extension !== config.extension) {
-      const extensionText = config.extension
-        ? ` from extension "${config.extension.name}"`
-        : '';
-      debugLogger.warn(
-        `Skipping MCP config for server with name "${name}"${extensionText} as it already exists.`,
-      );
-      return;
-    }
 
     const currentDiscoveryPromise = new Promise<void>((resolve, reject) => {
       (async () => {