Skip to content

Commit f5dd106

Browse files
LyalinDotComchrstnbgemini-code-assist[bot]
authored
fix(core): complete MCP discovery when configured servers are skipped (#18586)
Co-authored-by: christine betts <chrstn@uw.edu> Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
1 parent c202cf3 commit f5dd106

File tree

2 files changed

+46
-0
lines changed

2 files changed

+46
-0
lines changed

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

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,43 @@ describe('McpClientManager', () => {
103103
expect(manager.getDiscoveryState()).toBe(MCPDiscoveryState.COMPLETED);
104104
});
105105

106+
it('should mark discovery completed when all configured servers are user-disabled', async () => {
107+
mockConfig.getMcpServers.mockReturnValue({
108+
'test-server': {},
109+
});
110+
mockConfig.getMcpEnablementCallbacks.mockReturnValue({
111+
isSessionDisabled: vi.fn().mockReturnValue(false),
112+
isFileEnabled: vi.fn().mockResolvedValue(false),
113+
});
114+
115+
const manager = new McpClientManager('0.0.1', toolRegistry, mockConfig);
116+
const promise = manager.startConfiguredMcpServers();
117+
expect(manager.getDiscoveryState()).toBe(MCPDiscoveryState.IN_PROGRESS);
118+
await promise;
119+
120+
expect(manager.getDiscoveryState()).toBe(MCPDiscoveryState.COMPLETED);
121+
expect(manager.getMcpServerCount()).toBe(0);
122+
expect(mockedMcpClient.connect).not.toHaveBeenCalled();
123+
expect(mockedMcpClient.discover).not.toHaveBeenCalled();
124+
});
125+
126+
it('should mark discovery completed when all configured servers are blocked', async () => {
127+
mockConfig.getMcpServers.mockReturnValue({
128+
'test-server': {},
129+
});
130+
mockConfig.getBlockedMcpServers.mockReturnValue(['test-server']);
131+
132+
const manager = new McpClientManager('0.0.1', toolRegistry, mockConfig);
133+
const promise = manager.startConfiguredMcpServers();
134+
expect(manager.getDiscoveryState()).toBe(MCPDiscoveryState.IN_PROGRESS);
135+
await promise;
136+
137+
expect(manager.getDiscoveryState()).toBe(MCPDiscoveryState.COMPLETED);
138+
expect(manager.getMcpServerCount()).toBe(0);
139+
expect(mockedMcpClient.connect).not.toHaveBeenCalled();
140+
expect(mockedMcpClient.discover).not.toHaveBeenCalled();
141+
});
142+
106143
it('should not discover tools if folder is not trusted', async () => {
107144
mockConfig.getMcpServers.mockReturnValue({
108145
'test-server': {},

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -337,6 +337,15 @@ export class McpClientManager {
337337
this.maybeDiscoverMcpServer(name, config),
338338
),
339339
);
340+
341+
// If every configured server was skipped (for example because all are
342+
// disabled by user settings), no discovery promise is created. In that
343+
// case we must still mark discovery complete or the UI will wait forever.
344+
if (this.discoveryState === MCPDiscoveryState.IN_PROGRESS) {
345+
this.discoveryState = MCPDiscoveryState.COMPLETED;
346+
this.eventEmitter?.emit('mcp-client-update', this.clients);
347+
}
348+
340349
await this.cliConfig.refreshMcpContext();
341350
}
342351

0 commit comments

Comments
 (0)