Skip to content

Commit e7b20c4

Browse files
authored
Update mcp's list function to check for disablement. (#21148)
1 parent 743d05b commit e7b20c4

File tree

6 files changed

+193
-24
lines changed

6 files changed

+193
-24
lines changed

packages/cli/src/commands/mcp/list.test.ts

Lines changed: 71 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,16 @@ import {
1414
type Mock,
1515
} from 'vitest';
1616
import { listMcpServers } from './list.js';
17-
import { loadSettings, mergeSettings } from '../../config/settings.js';
17+
import {
18+
loadSettings,
19+
mergeSettings,
20+
type LoadedSettings,
21+
} from '../../config/settings.js';
1822
import { createTransport, debugLogger } from '@google/gemini-cli-core';
1923
import { Client } from '@modelcontextprotocol/sdk/client/index.js';
2024
import { ExtensionStorage } from '../../config/extensions/storage.js';
2125
import { ExtensionManager } from '../../config/extension-manager.js';
26+
import { McpServerEnablementManager } from '../../config/mcp/index.js';
2227

2328
vi.mock('../../config/settings.js', async (importOriginal) => {
2429
const actual =
@@ -45,6 +50,8 @@ vi.mock('@google/gemini-cli-core', async (importOriginal) => {
4550
CONNECTED: 'CONNECTED',
4651
CONNECTING: 'CONNECTING',
4752
DISCONNECTED: 'DISCONNECTED',
53+
BLOCKED: 'BLOCKED',
54+
DISABLED: 'DISABLED',
4855
},
4956
Storage: Object.assign(
5057
vi.fn().mockImplementation((_cwd: string) => ({
@@ -54,6 +61,7 @@ vi.mock('@google/gemini-cli-core', async (importOriginal) => {
5461
})),
5562
{
5663
getGlobalSettingsPath: () => '/tmp/gemini/settings.json',
64+
getGlobalGeminiDir: () => '/tmp/gemini',
5765
},
5866
),
5967
GEMINI_DIR: '.gemini',
@@ -96,6 +104,12 @@ describe('mcp list command', () => {
96104
beforeEach(() => {
97105
vi.resetAllMocks();
98106
vi.spyOn(debugLogger, 'log').mockImplementation(() => {});
107+
McpServerEnablementManager.resetInstance();
108+
// Use a mock for isFileEnabled to avoid reading real files
109+
vi.spyOn(
110+
McpServerEnablementManager.prototype,
111+
'isFileEnabled',
112+
).mockResolvedValue(true);
99113

100114
mockTransport = { close: vi.fn() };
101115
mockClient = {
@@ -265,7 +279,10 @@ describe('mcp list command', () => {
265279
mockClient.connect.mockResolvedValue(undefined);
266280
mockClient.ping.mockResolvedValue(undefined);
267281

268-
await listMcpServers(settingsWithAllowlist);
282+
await listMcpServers({
283+
merged: settingsWithAllowlist,
284+
isTrusted: true,
285+
} as unknown as LoadedSettings);
269286

270287
expect(debugLogger.log).toHaveBeenCalledWith(
271288
expect.stringContaining('allowed-server'),
@@ -304,4 +321,56 @@ describe('mcp list command', () => {
304321
),
305322
);
306323
});
324+
325+
it('should display blocked status for servers in excluded list', async () => {
326+
const defaultMergedSettings = mergeSettings({}, {}, {}, {}, true);
327+
mockedLoadSettings.mockReturnValue({
328+
merged: {
329+
...defaultMergedSettings,
330+
mcp: {
331+
excluded: ['blocked-server'],
332+
},
333+
mcpServers: {
334+
'blocked-server': { command: '/test/server' },
335+
},
336+
},
337+
isTrusted: true,
338+
});
339+
340+
await listMcpServers();
341+
342+
expect(debugLogger.log).toHaveBeenCalledWith(
343+
expect.stringContaining(
344+
'blocked-server: /test/server (stdio) - Blocked',
345+
),
346+
);
347+
expect(mockedCreateTransport).not.toHaveBeenCalled();
348+
});
349+
350+
it('should display disabled status for servers disabled via enablement manager', async () => {
351+
const defaultMergedSettings = mergeSettings({}, {}, {}, {}, true);
352+
mockedLoadSettings.mockReturnValue({
353+
merged: {
354+
...defaultMergedSettings,
355+
mcpServers: {
356+
'disabled-server': { command: '/test/server' },
357+
},
358+
},
359+
isTrusted: true,
360+
});
361+
362+
vi.spyOn(
363+
McpServerEnablementManager.prototype,
364+
'isFileEnabled',
365+
).mockResolvedValue(false);
366+
367+
await listMcpServers();
368+
369+
expect(debugLogger.log).toHaveBeenCalledWith(
370+
expect.stringContaining(
371+
'disabled-server: /test/server (stdio) - Disabled',
372+
),
373+
);
374+
expect(mockedCreateTransport).not.toHaveBeenCalled();
375+
});
307376
});

packages/cli/src/commands/mcp/list.ts

Lines changed: 60 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -6,17 +6,25 @@
66

77
// File for 'gemini mcp list' command
88
import type { CommandModule } from 'yargs';
9-
import { type MergedSettings, loadSettings } from '../../config/settings.js';
10-
import type { MCPServerConfig } from '@google/gemini-cli-core';
9+
import {
10+
type MergedSettings,
11+
loadSettings,
12+
type LoadedSettings,
13+
} from '../../config/settings.js';
1114
import {
1215
MCPServerStatus,
1316
createTransport,
1417
debugLogger,
1518
applyAdminAllowlist,
1619
getAdminBlockedMcpServersMessage,
1720
} from '@google/gemini-cli-core';
21+
import type { MCPServerConfig } from '@google/gemini-cli-core';
1822
import { Client } from '@modelcontextprotocol/sdk/client/index.js';
1923
import { ExtensionManager } from '../../config/extension-manager.js';
24+
import {
25+
canLoadServer,
26+
McpServerEnablementManager,
27+
} from '../../config/mcp/index.js';
2028
import { requestConsentNonInteractive } from '../../config/extensions/consent.js';
2129
import { promptForSetting } from '../../config/extensions/extensionSettings.js';
2230
import { exitCli } from '../utils.js';
@@ -61,13 +69,13 @@ export async function getMcpServersFromConfig(
6169
async function testMCPConnection(
6270
serverName: string,
6371
config: MCPServerConfig,
72+
isTrusted: boolean,
73+
activeSettings: MergedSettings,
6474
): Promise<MCPServerStatus> {
65-
const settings = loadSettings();
66-
6775
// SECURITY: Only test connection if workspace is trusted or if it's a remote server.
6876
// stdio servers execute local commands and must never run in untrusted workspaces.
6977
const isStdio = !!config.command;
70-
if (isStdio && !settings.isTrusted) {
78+
if (isStdio && !isTrusted) {
7179
return MCPServerStatus.DISCONNECTED;
7280
}
7381

@@ -80,7 +88,7 @@ async function testMCPConnection(
8088
sanitizationConfig: {
8189
enableEnvironmentVariableRedaction: true,
8290
allowedEnvironmentVariables: [],
83-
blockedEnvironmentVariables: settings.merged.advanced.excludedEnvVars,
91+
blockedEnvironmentVariables: activeSettings.advanced.excludedEnvVars,
8492
},
8593
emitMcpDiagnostic: (
8694
severity: 'info' | 'warning' | 'error',
@@ -105,7 +113,7 @@ async function testMCPConnection(
105113
debugLogger.log(message, error);
106114
}
107115
},
108-
isTrustedFolder: () => settings.isTrusted,
116+
isTrustedFolder: () => isTrusted,
109117
};
110118

111119
let transport;
@@ -135,14 +143,40 @@ async function testMCPConnection(
135143
async function getServerStatus(
136144
serverName: string,
137145
server: MCPServerConfig,
146+
isTrusted: boolean,
147+
activeSettings: MergedSettings,
138148
): Promise<MCPServerStatus> {
149+
const mcpEnablementManager = McpServerEnablementManager.getInstance();
150+
const loadResult = await canLoadServer(serverName, {
151+
adminMcpEnabled: activeSettings.admin?.mcp?.enabled ?? true,
152+
allowedList: activeSettings.mcp?.allowed,
153+
excludedList: activeSettings.mcp?.excluded,
154+
enablement: mcpEnablementManager.getEnablementCallbacks(),
155+
});
156+
157+
if (!loadResult.allowed) {
158+
if (
159+
loadResult.blockType === 'admin' ||
160+
loadResult.blockType === 'allowlist' ||
161+
loadResult.blockType === 'excludelist'
162+
) {
163+
return MCPServerStatus.BLOCKED;
164+
}
165+
return MCPServerStatus.DISABLED;
166+
}
167+
139168
// Test all server types by attempting actual connection
140-
return testMCPConnection(serverName, server);
169+
return testMCPConnection(serverName, server, isTrusted, activeSettings);
141170
}
142171

143-
export async function listMcpServers(settings?: MergedSettings): Promise<void> {
172+
export async function listMcpServers(
173+
loadedSettingsArg?: LoadedSettings,
174+
): Promise<void> {
175+
const loadedSettings = loadedSettingsArg ?? loadSettings();
176+
const activeSettings = loadedSettings.merged;
177+
144178
const { mcpServers, blockedServerNames } =
145-
await getMcpServersFromConfig(settings);
179+
await getMcpServersFromConfig(activeSettings);
146180
const serverNames = Object.keys(mcpServers);
147181

148182
if (blockedServerNames.length > 0) {
@@ -165,7 +199,12 @@ export async function listMcpServers(settings?: MergedSettings): Promise<void> {
165199
for (const serverName of serverNames) {
166200
const server = mcpServers[serverName];
167201

168-
const status = await getServerStatus(serverName, server);
202+
const status = await getServerStatus(
203+
serverName,
204+
server,
205+
loadedSettings.isTrusted,
206+
activeSettings,
207+
);
169208

170209
let statusIndicator = '';
171210
let statusText = '';
@@ -178,6 +217,14 @@ export async function listMcpServers(settings?: MergedSettings): Promise<void> {
178217
statusIndicator = chalk.yellow('…');
179218
statusText = 'Connecting';
180219
break;
220+
case MCPServerStatus.BLOCKED:
221+
statusIndicator = chalk.red('⛔');
222+
statusText = 'Blocked';
223+
break;
224+
case MCPServerStatus.DISABLED:
225+
statusIndicator = chalk.gray('○');
226+
statusText = 'Disabled';
227+
break;
181228
case MCPServerStatus.DISCONNECTED:
182229
default:
183230
statusIndicator = chalk.red('✗');
@@ -203,14 +250,14 @@ export async function listMcpServers(settings?: MergedSettings): Promise<void> {
203250
}
204251

205252
interface ListArgs {
206-
settings?: MergedSettings;
253+
loadedSettings?: LoadedSettings;
207254
}
208255

209256
export const listCommand: CommandModule<object, ListArgs> = {
210257
command: 'list',
211258
describe: 'List all configured MCP servers',
212259
handler: async (argv) => {
213-
await listMcpServers(argv.settings);
260+
await listMcpServers(argv.loadedSettings);
214261
await exitCli();
215262
},
216263
};

packages/cli/src/ui/components/views/McpStatus.test.tsx

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ describe('McpStatus', () => {
1616
servers: {
1717
'server-1': {
1818
url: 'http://localhost:8080',
19-
name: 'server-1',
2019
description: 'A test server',
2120
},
2221
},
@@ -200,6 +199,38 @@ describe('McpStatus', () => {
200199
unmount();
201200
});
202201

202+
it('renders correctly with both blocked and unblocked servers', async () => {
203+
const { lastFrame, unmount, waitUntilReady } = render(
204+
<McpStatus
205+
{...baseProps}
206+
servers={{
207+
...baseProps.servers,
208+
'server-2': {
209+
url: 'http://localhost:8081',
210+
description: 'A blocked server',
211+
},
212+
}}
213+
blockedServers={[{ name: 'server-2', extensionName: 'test-extension' }]}
214+
/>,
215+
);
216+
await waitUntilReady();
217+
expect(lastFrame()).toMatchSnapshot();
218+
unmount();
219+
});
220+
221+
it('renders only blocked servers when no configured servers exist', async () => {
222+
const { lastFrame, unmount, waitUntilReady } = render(
223+
<McpStatus
224+
{...baseProps}
225+
servers={{}}
226+
blockedServers={[{ name: 'server-1', extensionName: 'test-extension' }]}
227+
/>,
228+
);
229+
await waitUntilReady();
230+
expect(lastFrame()).toMatchSnapshot();
231+
unmount();
232+
});
233+
203234
it('renders correctly with a connecting server', async () => {
204235
const { lastFrame, unmount, waitUntilReady } = render(
205236
<McpStatus {...baseProps} connectingServers={['server-1']} />,

packages/cli/src/ui/components/views/McpStatus.tsx

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,12 @@ export const McpStatus: React.FC<McpStatusProps> = ({
4848
showDescriptions,
4949
showSchema,
5050
}) => {
51-
const serverNames = Object.keys(servers);
51+
const serverNames = Object.keys(servers).filter(
52+
(serverName) =>
53+
!blockedServers.some(
54+
(blockedServer) => blockedServer.name === serverName,
55+
),
56+
);
5257

5358
if (serverNames.length === 0 && blockedServers.length === 0) {
5459
return (
@@ -82,7 +87,6 @@ export const McpStatus: React.FC<McpStatusProps> = ({
8287

8388
<Text bold>Configured MCP servers:</Text>
8489
<Box height={1} />
85-
8690
{serverNames.map((serverName) => {
8791
const server = servers[serverName];
8892
const serverTools = tools.filter(

packages/cli/src/ui/components/views/__snapshots__/McpStatus.test.tsx.snap

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,6 @@ A test server
1717
exports[`McpStatus > renders correctly with a blocked server 1`] = `
1818
"Configured MCP servers:
1919
20-
🟢 server-1 - Ready (1 tool)
21-
A test server
22-
Tools:
23-
- tool-1
24-
A test tool
25-
2620
🔴 server-1 (from test-extension) - Blocked
2721
"
2822
`;
@@ -83,6 +77,19 @@ A test server
8377
"
8478
`;
8579

80+
exports[`McpStatus > renders correctly with both blocked and unblocked servers 1`] = `
81+
"Configured MCP servers:
82+
83+
🟢 server-1 - Ready (1 tool)
84+
A test server
85+
Tools:
86+
- tool-1
87+
A test tool
88+
89+
🔴 server-2 (from test-extension) - Blocked
90+
"
91+
`;
92+
8693
exports[`McpStatus > renders correctly with expired OAuth status 1`] = `
8794
"Configured MCP servers:
8895
@@ -172,3 +179,10 @@ A test server
172179
A test tool
173180
"
174181
`;
182+
183+
exports[`McpStatus > renders only blocked servers when no configured servers exist 1`] = `
184+
"Configured MCP servers:
185+
186+
🔴 server-1 (from test-extension) - Blocked
187+
"
188+
`;

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,10 @@ export enum MCPServerStatus {
104104
CONNECTING = 'connecting',
105105
/** Server is connected and ready to use */
106106
CONNECTED = 'connected',
107+
/** Server is blocked via configuration and cannot be used */
108+
BLOCKED = 'blocked',
109+
/** Server is disabled and cannot be used */
110+
DISABLED = 'disabled',
107111
}
108112

109113
/**

0 commit comments

Comments
 (0)