mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-04-20 10:10:56 -07:00
Add support for running available commands prior to MCP servers loading (#15596)
This commit is contained in:
@@ -28,6 +28,9 @@ import {
|
||||
ToolConfirmationOutcome,
|
||||
Storage,
|
||||
IdeClient,
|
||||
addMCPStatusChangeListener,
|
||||
removeMCPStatusChangeListener,
|
||||
MCPDiscoveryState,
|
||||
} from '@google/gemini-cli-core';
|
||||
import { useSessionStats } from '../contexts/SessionContext.js';
|
||||
import type {
|
||||
@@ -269,6 +272,10 @@ export const useSlashCommandProcessor = (
|
||||
ideClient.addStatusChangeListener(listener);
|
||||
})();
|
||||
|
||||
// Listen for MCP server status changes (e.g. connection, discovery completion)
|
||||
// to reload slash commands (since they may include MCP prompts).
|
||||
addMCPStatusChangeListener(listener);
|
||||
|
||||
// TODO: Ideally this would happen more directly inside the ExtensionLoader,
|
||||
// but the CommandService today is not conducive to that since it isn't a
|
||||
// long lived service but instead gets fully re-created based on reload
|
||||
@@ -289,6 +296,7 @@ export const useSlashCommandProcessor = (
|
||||
const ideClient = await IdeClient.getInstance();
|
||||
ideClient.removeStatusChangeListener(listener);
|
||||
})();
|
||||
removeMCPStatusChangeListener(listener);
|
||||
appEvents.off('extensionsStarting', extensionEventListener);
|
||||
appEvents.off('extensionsStopping', extensionEventListener);
|
||||
};
|
||||
@@ -572,9 +580,16 @@ export const useSlashCommandProcessor = (
|
||||
}
|
||||
}
|
||||
|
||||
const isMcpLoading =
|
||||
config?.getMcpClientManager()?.getDiscoveryState() ===
|
||||
MCPDiscoveryState.IN_PROGRESS;
|
||||
const errorMessage = isMcpLoading
|
||||
? `Unknown command: ${trimmed}. Command might have been from an MCP server but MCP servers are not done loading.`
|
||||
: `Unknown command: ${trimmed}`;
|
||||
|
||||
addMessage({
|
||||
type: MessageType.ERROR,
|
||||
content: `Unknown command: ${trimmed}`,
|
||||
content: errorMessage,
|
||||
timestamp: new Date(),
|
||||
});
|
||||
|
||||
|
||||
@@ -36,6 +36,7 @@ import {
|
||||
debugLogger,
|
||||
coreEvents,
|
||||
CoreEvent,
|
||||
MCPDiscoveryState,
|
||||
} from '@google/gemini-cli-core';
|
||||
import type { Part, PartListUnion } from '@google/genai';
|
||||
import type { UseHistoryManagerReturn } from './useHistoryManager.js';
|
||||
@@ -178,6 +179,11 @@ describe('useGeminiStream', () => {
|
||||
return clientInstance;
|
||||
});
|
||||
|
||||
const mockMcpClientManager = {
|
||||
getDiscoveryState: vi.fn().mockReturnValue(MCPDiscoveryState.COMPLETED),
|
||||
getMcpServerCount: vi.fn().mockReturnValue(0),
|
||||
};
|
||||
|
||||
const contentGeneratorConfig = {
|
||||
model: 'test-model',
|
||||
apiKey: 'test-key',
|
||||
@@ -211,6 +217,7 @@ describe('useGeminiStream', () => {
|
||||
getProjectRoot: vi.fn(() => '/test/dir'),
|
||||
getCheckpointingEnabled: vi.fn(() => false),
|
||||
getGeminiClient: mockGetGeminiClient,
|
||||
getMcpClientManager: () => mockMcpClientManager as any,
|
||||
getApprovalMode: () => ApprovalMode.DEFAULT,
|
||||
getUsageStatisticsEnabled: () => true,
|
||||
getDebugMode: () => false,
|
||||
@@ -254,6 +261,7 @@ describe('useGeminiStream', () => {
|
||||
.mockClear()
|
||||
.mockReturnValue((async function* () {})());
|
||||
handleAtCommandSpy = vi.spyOn(atCommandProcessor, 'handleAtCommand');
|
||||
vi.spyOn(coreEvents, 'emitFeedback');
|
||||
});
|
||||
|
||||
const mockLoadedSettings: LoadedSettings = {
|
||||
@@ -1954,6 +1962,73 @@ describe('useGeminiStream', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('MCP Discovery State', () => {
|
||||
it('should block non-slash command queries when discovery is in progress and servers exist', async () => {
|
||||
const mockMcpClientManager = {
|
||||
getDiscoveryState: vi
|
||||
.fn()
|
||||
.mockReturnValue(MCPDiscoveryState.IN_PROGRESS),
|
||||
getMcpServerCount: vi.fn().mockReturnValue(1),
|
||||
};
|
||||
mockConfig.getMcpClientManager = () => mockMcpClientManager as any;
|
||||
|
||||
const { result } = renderTestHook();
|
||||
|
||||
await act(async () => {
|
||||
await result.current.submitQuery('test query');
|
||||
});
|
||||
|
||||
expect(coreEvents.emitFeedback).toHaveBeenCalledWith(
|
||||
'info',
|
||||
'Waiting for MCP servers to initialize... Slash commands are still available.',
|
||||
);
|
||||
expect(mockSendMessageStream).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should NOT block queries when discovery is NOT_STARTED but there are no servers', async () => {
|
||||
const mockMcpClientManager = {
|
||||
getDiscoveryState: vi
|
||||
.fn()
|
||||
.mockReturnValue(MCPDiscoveryState.NOT_STARTED),
|
||||
getMcpServerCount: vi.fn().mockReturnValue(0),
|
||||
};
|
||||
mockConfig.getMcpClientManager = () => mockMcpClientManager as any;
|
||||
|
||||
const { result } = renderTestHook();
|
||||
|
||||
await act(async () => {
|
||||
await result.current.submitQuery('test query');
|
||||
});
|
||||
|
||||
expect(coreEvents.emitFeedback).not.toHaveBeenCalledWith(
|
||||
'info',
|
||||
'Waiting for MCP servers to initialize... Slash commands are still available.',
|
||||
);
|
||||
expect(mockSendMessageStream).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should NOT block slash commands even when discovery is in progress', async () => {
|
||||
const mockMcpClientManager = {
|
||||
getDiscoveryState: vi
|
||||
.fn()
|
||||
.mockReturnValue(MCPDiscoveryState.IN_PROGRESS),
|
||||
getMcpServerCount: vi.fn().mockReturnValue(1),
|
||||
};
|
||||
mockConfig.getMcpClientManager = () => mockMcpClientManager as any;
|
||||
|
||||
const { result } = renderTestHook();
|
||||
|
||||
await act(async () => {
|
||||
await result.current.submitQuery('/help');
|
||||
});
|
||||
|
||||
expect(coreEvents.emitFeedback).not.toHaveBeenCalledWith(
|
||||
'info',
|
||||
'Waiting for MCP servers to initialize... Slash commands are still available.',
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('handleFinishedEvent', () => {
|
||||
it('should add info message for MAX_TOKENS finish reason', async () => {
|
||||
// Setup mock to return a stream with MAX_TOKENS finish reason
|
||||
@@ -3015,4 +3090,68 @@ describe('useGeminiStream', () => {
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('MCP Server Initialization', () => {
|
||||
it('should allow slash commands to run while MCP servers are initializing', async () => {
|
||||
const mockMcpClientManager = {
|
||||
getDiscoveryState: vi
|
||||
.fn()
|
||||
.mockReturnValue(MCPDiscoveryState.IN_PROGRESS),
|
||||
getMcpServerCount: vi.fn().mockReturnValue(1),
|
||||
};
|
||||
mockConfig.getMcpClientManager = () => mockMcpClientManager as any;
|
||||
|
||||
const { result } = renderTestHook();
|
||||
|
||||
await act(async () => {
|
||||
await result.current.submitQuery('/help');
|
||||
});
|
||||
|
||||
// Slash command should be handled, and no Gemini call should be made.
|
||||
expect(mockHandleSlashCommand).toHaveBeenCalledWith('/help');
|
||||
expect(coreEvents.emitFeedback).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should block normal prompts and provide feedback while MCP servers are initializing', async () => {
|
||||
const mockMcpClientManager = {
|
||||
getDiscoveryState: vi
|
||||
.fn()
|
||||
.mockReturnValue(MCPDiscoveryState.IN_PROGRESS),
|
||||
getMcpServerCount: vi.fn().mockReturnValue(1),
|
||||
};
|
||||
mockConfig.getMcpClientManager = () => mockMcpClientManager as any;
|
||||
const { result } = renderTestHook();
|
||||
|
||||
await act(async () => {
|
||||
await result.current.submitQuery('a normal prompt');
|
||||
});
|
||||
|
||||
// No slash command, no Gemini call, but feedback should be emitted.
|
||||
expect(mockHandleSlashCommand).not.toHaveBeenCalled();
|
||||
expect(mockSendMessageStream).not.toHaveBeenCalled();
|
||||
expect(coreEvents.emitFeedback).toHaveBeenCalledWith(
|
||||
'info',
|
||||
'Waiting for MCP servers to initialize... Slash commands are still available.',
|
||||
);
|
||||
});
|
||||
|
||||
it('should allow normal prompts to run when MCP servers are finished initializing', async () => {
|
||||
const mockMcpClientManager = {
|
||||
getDiscoveryState: vi.fn().mockReturnValue(MCPDiscoveryState.COMPLETED),
|
||||
getMcpServerCount: vi.fn().mockReturnValue(1),
|
||||
};
|
||||
mockConfig.getMcpClientManager = () => mockMcpClientManager as any;
|
||||
|
||||
const { result } = renderTestHook();
|
||||
|
||||
await act(async () => {
|
||||
await result.current.submitQuery('a normal prompt');
|
||||
});
|
||||
|
||||
// Prompt should be sent to Gemini.
|
||||
expect(mockHandleSlashCommand).not.toHaveBeenCalled();
|
||||
expect(mockSendMessageStream).toHaveBeenCalled();
|
||||
expect(coreEvents.emitFeedback).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -30,6 +30,7 @@ import {
|
||||
ToolErrorType,
|
||||
coreEvents,
|
||||
CoreEvent,
|
||||
MCPDiscoveryState,
|
||||
} from '@google/gemini-cli-core';
|
||||
import type {
|
||||
Config,
|
||||
@@ -951,6 +952,26 @@ export const useGeminiStream = (
|
||||
{ name: 'submitQuery' },
|
||||
async ({ metadata: spanMetadata }) => {
|
||||
spanMetadata.input = query;
|
||||
|
||||
const discoveryState = config
|
||||
.getMcpClientManager()
|
||||
?.getDiscoveryState();
|
||||
const mcpServerCount =
|
||||
config.getMcpClientManager()?.getMcpServerCount() ?? 0;
|
||||
if (
|
||||
!options?.isContinuation &&
|
||||
typeof query === 'string' &&
|
||||
!isSlashCommand(query.trim()) &&
|
||||
mcpServerCount > 0 &&
|
||||
discoveryState !== MCPDiscoveryState.COMPLETED
|
||||
) {
|
||||
coreEvents.emitFeedback(
|
||||
'info',
|
||||
'Waiting for MCP servers to initialize... Slash commands are still available.',
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
const queryId = `${Date.now()}-${Math.random()}`;
|
||||
activeQueryIdRef.current = queryId;
|
||||
if (
|
||||
|
||||
@@ -274,6 +274,35 @@ describe('Server Config (config.ts)', () => {
|
||||
);
|
||||
});
|
||||
|
||||
it('should not await MCP initialization', async () => {
|
||||
const config = new Config({
|
||||
...baseParams,
|
||||
checkpointing: false,
|
||||
});
|
||||
|
||||
const { McpClientManager } = await import(
|
||||
'../tools/mcp-client-manager.js'
|
||||
);
|
||||
let mcpStarted = false;
|
||||
|
||||
(McpClientManager as unknown as Mock).mockImplementation(() => ({
|
||||
startConfiguredMcpServers: vi.fn().mockImplementation(async () => {
|
||||
await new Promise((resolve) => setTimeout(resolve, 50));
|
||||
mcpStarted = true;
|
||||
}),
|
||||
getMcpInstructions: vi.fn(),
|
||||
}));
|
||||
|
||||
await config.initialize();
|
||||
|
||||
// Should return immediately, before MCP finishes (50ms delay)
|
||||
expect(mcpStarted).toBe(false);
|
||||
|
||||
// Wait for it to eventually finish to avoid open handles
|
||||
await new Promise((resolve) => setTimeout(resolve, 60));
|
||||
expect(mcpStarted).toBe(true);
|
||||
});
|
||||
|
||||
describe('getCompressionThreshold', () => {
|
||||
it('should return the local compression threshold if it is set', async () => {
|
||||
const config = new Config({
|
||||
|
||||
@@ -796,12 +796,14 @@ export class Config {
|
||||
this,
|
||||
this.eventEmitter,
|
||||
);
|
||||
const initMcpHandle = startupProfiler.start('initialize_mcp_clients');
|
||||
await Promise.all([
|
||||
await this.mcpClientManager.startConfiguredMcpServers(),
|
||||
await this.getExtensionLoader().start(this),
|
||||
]);
|
||||
initMcpHandle?.end();
|
||||
// We do not await this promise so that the CLI can start up even if
|
||||
// MCP servers are slow to connect.
|
||||
Promise.all([
|
||||
this.mcpClientManager.startConfiguredMcpServers(),
|
||||
this.getExtensionLoader().start(this),
|
||||
]).catch((error) => {
|
||||
debugLogger.error('Error initializing MCP clients:', error);
|
||||
});
|
||||
|
||||
if (this.skillsSupport) {
|
||||
this.getSkillManager().setAdminSettings(this.adminSkillsEnabled);
|
||||
|
||||
@@ -14,7 +14,7 @@ import {
|
||||
type MockedObject,
|
||||
} from 'vitest';
|
||||
import { McpClientManager } from './mcp-client-manager.js';
|
||||
import { McpClient } from './mcp-client.js';
|
||||
import { McpClient, MCPDiscoveryState } from './mcp-client.js';
|
||||
import type { ToolRegistry } from './tool-registry.js';
|
||||
import type { Config } from '../config/config.js';
|
||||
|
||||
@@ -71,6 +71,18 @@ describe('McpClientManager', () => {
|
||||
expect(mockedMcpClient.discover).toHaveBeenCalledOnce();
|
||||
});
|
||||
|
||||
it('should update global discovery state', async () => {
|
||||
mockConfig.getMcpServers.mockReturnValue({
|
||||
'test-server': {},
|
||||
});
|
||||
const manager = new McpClientManager(toolRegistry, mockConfig);
|
||||
expect(manager.getDiscoveryState()).toBe(MCPDiscoveryState.NOT_STARTED);
|
||||
const promise = manager.startConfiguredMcpServers();
|
||||
expect(manager.getDiscoveryState()).toBe(MCPDiscoveryState.IN_PROGRESS);
|
||||
await promise;
|
||||
expect(manager.getDiscoveryState()).toBe(MCPDiscoveryState.COMPLETED);
|
||||
});
|
||||
|
||||
it('should not discover tools if folder is not trusted', async () => {
|
||||
mockConfig.getMcpServers.mockReturnValue({
|
||||
'test-server': {},
|
||||
|
||||
@@ -362,4 +362,8 @@ export class McpClientManager {
|
||||
}
|
||||
return instructions.join('\n\n');
|
||||
}
|
||||
|
||||
getMcpServerCount(): number {
|
||||
return this.clients.size;
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user