Skip to content

fix(cli): improve mcp list UX in untrusted folders #26457

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Adib234 merged 3 commits into from
May 6, 2026
+157 −97
Merged

fix(cli): improve mcp list UX in untrusted folders #26457

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e1d9022
fix(cli): improve mcp list UX in untrusted folders
Adib234 May 4, 2026
e629fbe
Update packages/cli/src/commands/mcp/list.ts
Adib234 May 4, 2026
3bb1315
test(cli): refactor mcp list tests to use createMockSettings and remo…
Adib234 May 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
215 changes: 122 additions & 93 deletions packages/cli/src/commands/mcp/list.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,17 @@ import {
type Mock,
} from 'vitest';
import { listMcpServers } from './list.js';
import { loadSettings } from '../../config/settings.js';
import {
loadSettings,
mergeSettings,
type LoadedSettings,
} from '../../config/settings.js';
import { createTransport, debugLogger } from '@google/gemini-cli-core';
createTransport,
debugLogger,
type AdminControlsSettings,
} from '@google/gemini-cli-core';
import { Client } from '@modelcontextprotocol/sdk/client/index.js';
import { ExtensionStorage } from '../../config/extensions/storage.js';
import { ExtensionManager } from '../../config/extension-manager.js';
import { McpServerEnablementManager } from '../../config/mcp/index.js';
import { createMockSettings } from '../../test-utils/settings.js';

vi.mock('../../config/settings.js', async (importOriginal) => {
const actual =
Expand Down Expand Up @@ -133,21 +134,16 @@ describe('mcp list command', () => {
});

it('should display message when no servers configured', async () => {
const defaultMergedSettings = mergeSettings({}, {}, {}, {}, true);
mockedLoadSettings.mockReturnValue({
merged: { ...defaultMergedSettings, mcpServers: {} },
});
mockedLoadSettings.mockReturnValue(createMockSettings({ mcpServers: {} }));

await listMcpServers();

expect(debugLogger.log).toHaveBeenCalledWith('No MCP servers configured.');
});

it('should display different server types with connected status', async () => {
const defaultMergedSettings = mergeSettings({}, {}, {}, {}, true);
mockedLoadSettings.mockReturnValue({
merged: {
...defaultMergedSettings,
mockedLoadSettings.mockReturnValue(
createMockSettings({
mcpServers: {
'stdio-server': { command: '/path/to/server', args: ['arg1'] },
'sse-server': { url: 'https://example.com/sse', type: 'sse' },
Expand All @@ -158,9 +154,9 @@ describe('mcp list command', () => {
type: 'http',
},
},
},
isTrusted: true,
});
isTrusted: true,
}),
);

mockClient.connect.mockResolvedValue(undefined);
mockClient.ping.mockResolvedValue(undefined);
Expand Down Expand Up @@ -196,15 +192,14 @@ describe('mcp list command', () => {
});

it('should display disconnected status when connection fails', async () => {
const defaultMergedSettings = mergeSettings({}, {}, {}, {}, true);
mockedLoadSettings.mockReturnValue({
merged: {
...defaultMergedSettings,
mockedLoadSettings.mockReturnValue(
createMockSettings({
mcpServers: {
'test-server': { command: '/test/server' },
},
},
});
isTrusted: true,
}),
);

mockClient.connect.mockRejectedValue(new Error('Connection failed'));

Expand All @@ -218,16 +213,14 @@ describe('mcp list command', () => {
});

it('should display connected status even if ping fails', async () => {
const defaultMergedSettings = mergeSettings({}, {}, {}, {}, true);
mockedLoadSettings.mockReturnValue({
merged: {
...defaultMergedSettings,
mockedLoadSettings.mockReturnValue(
createMockSettings({
mcpServers: {
'test-server': { command: '/test/server' },
},
},
isTrusted: true,
});
isTrusted: true,
}),
);

mockClient.connect.mockResolvedValue(undefined);
mockClient.ping.mockRejectedValue(new Error('Ping failed'));
Expand All @@ -240,16 +233,14 @@ describe('mcp list command', () => {
});

it('should use configured timeout for connection', async () => {
const defaultMergedSettings = mergeSettings({}, {}, {}, {}, true);
mockedLoadSettings.mockReturnValue({
merged: {
...defaultMergedSettings,
mockedLoadSettings.mockReturnValue(
createMockSettings({
mcpServers: {
'test-server': { command: '/test/server', timeout: 12345 },
},
},
isTrusted: true,
});
isTrusted: true,
}),
);

mockClient.connect.mockResolvedValue(undefined);

Expand All @@ -265,16 +256,14 @@ describe('mcp list command', () => {
});

it('should use default timeout for connection when not configured', async () => {
const defaultMergedSettings = mergeSettings({}, {}, {}, {}, true);
mockedLoadSettings.mockReturnValue({
merged: {
...defaultMergedSettings,
mockedLoadSettings.mockReturnValue(
createMockSettings({
mcpServers: {
'test-server': { command: '/test/server' },
},
},
isTrusted: true,
});
isTrusted: true,
}),
);

mockClient.connect.mockResolvedValue(undefined);

Expand All @@ -290,16 +279,14 @@ describe('mcp list command', () => {
});

it('should merge extension servers with config servers', async () => {
const defaultMergedSettings = mergeSettings({}, {}, {}, {}, true);
mockedLoadSettings.mockReturnValue({
merged: {
...defaultMergedSettings,
mockedLoadSettings.mockReturnValue(
createMockSettings({
mcpServers: {
'config-server': { command: '/config/server' },
},
},
isTrusted: true,
});
isTrusted: true,
}),
);

mockExtensionManager.loadExtensions.mockReturnValue([
{
Expand All @@ -326,36 +313,40 @@ describe('mcp list command', () => {
});

it('should filter servers based on admin allowlist passed in settings', async () => {
const settingsWithAllowlist = mergeSettings({}, {}, {}, {}, true);
settingsWithAllowlist.admin = {
secureModeEnabled: false,
extensions: { enabled: true },
skills: { enabled: true },
mcp: {
enabled: true,
config: {
'allowed-server': { url: 'http://allowed' },
const adminControls = {
strictModeDisabled: true,
mcpSetting: {
mcpEnabled: true,
mcpConfig: {
mcpServers: {
'allowed-server': { url: 'http://allowed' },
},
},
requiredConfig: {},
},
};

settingsWithAllowlist.mcpServers = {
const mcpServers = {
'allowed-server': { command: 'cmd1' },
'forbidden-server': { command: 'cmd2' },
};

mockedLoadSettings.mockReturnValue({
merged: settingsWithAllowlist,
const mockSettings = createMockSettings({
mcpServers,
isTrusted: true,
});
// setRemoteAdminSettings is the correct way to set admin settings in tests
(
mockSettings as unknown as {
setRemoteAdminSettings: (controls: AdminControlsSettings) => void;
}
).setRemoteAdminSettings(adminControls as unknown as AdminControlsSettings);

mockedLoadSettings.mockReturnValue(mockSettings);

mockClient.connect.mockResolvedValue(undefined);
mockClient.ping.mockResolvedValue(undefined);

await listMcpServers({
merged: settingsWithAllowlist,
isTrusted: true,
} as unknown as LoadedSettings);
await listMcpServers(mockSettings);

expect(debugLogger.log).toHaveBeenCalledWith(
expect.stringContaining('allowed-server'),
Expand All @@ -371,44 +362,40 @@ describe('mcp list command', () => {
);
});

it('should show stdio servers as disconnected in untrusted folders', async () => {
const defaultMergedSettings = mergeSettings({}, {}, {}, {}, true);
mockedLoadSettings.mockReturnValue({
merged: {
...defaultMergedSettings,
it('should show stdio servers as disabled in untrusted folders', async () => {
mockedLoadSettings.mockReturnValue(
createMockSettings({
mcpServers: {
'test-server': { command: '/test/server' },
},
},
isTrusted: false,
});

// createTransport will throw in core if not trusted
mockedCreateTransport.mockRejectedValue(new Error('Folder not trusted'));
isTrusted: false,
}),
);

await listMcpServers();

expect(debugLogger.log).toHaveBeenCalledWith(
expect.stringContaining(
'test-server: /test/server (stdio) - Disconnected',
'Warning: MCP servers are configured but disabled because this folder is untrusted.',
),
);
expect(debugLogger.log).toHaveBeenCalledWith(
expect.stringContaining('test-server: /test/server (stdio) - Disabled'),
);
});

it('should display blocked status for servers in excluded list', async () => {
const defaultMergedSettings = mergeSettings({}, {}, {}, {}, true);
mockedLoadSettings.mockReturnValue({
merged: {
...defaultMergedSettings,
mockedLoadSettings.mockReturnValue(
createMockSettings({
mcp: {
excluded: ['blocked-server'],
},
mcpServers: {
'blocked-server': { command: '/test/server' },
},
},
isTrusted: true,
});
isTrusted: true,
}),
);

await listMcpServers();

Expand All @@ -421,16 +408,14 @@ describe('mcp list command', () => {
});

it('should display disabled status for servers disabled via enablement manager', async () => {
const defaultMergedSettings = mergeSettings({}, {}, {}, {}, true);
mockedLoadSettings.mockReturnValue({
merged: {
...defaultMergedSettings,
mockedLoadSettings.mockReturnValue(
createMockSettings({
mcpServers: {
'disabled-server': { command: '/test/server' },
},
},
isTrusted: true,
});
isTrusted: true,
}),
);

vi.spyOn(
McpServerEnablementManager.prototype,
Expand All @@ -446,4 +431,48 @@ describe('mcp list command', () => {
);
expect(mockedCreateTransport).not.toHaveBeenCalled();
});

it('should display warning and disabled status in untrusted folders', async () => {
const userMcpServers = {
'user-server': { url: 'https://example.com/user' },
};
const workspaceMcpServers = {
'project-server': { command: '/path/to/project/server' },
};

mockedLoadSettings.mockReturnValue(
createMockSettings({
user: {
settings: { mcpServers: userMcpServers },
originalSettings: { mcpServers: userMcpServers },
path: '/mock/user/settings.json',
},
workspace: {
settings: { mcpServers: workspaceMcpServers },
originalSettings: { mcpServers: workspaceMcpServers },
path: '/mock/workspace/settings.json',
},
isTrusted: false,
}),
);

await listMcpServers();

expect(debugLogger.log).toHaveBeenCalledWith(
expect.stringContaining(
'Warning: MCP servers are configured but disabled because this folder is untrusted.',
),
);
expect(debugLogger.log).toHaveBeenCalledWith(
expect.stringContaining(
'project-server: /path/to/project/server (stdio) - Disabled',
),
);
expect(debugLogger.log).toHaveBeenCalledWith(
expect.stringContaining(
'user-server: https://example.com/user (http) - Disabled',
),
);
expect(mockedCreateTransport).not.toHaveBeenCalled();
});
});
Loading
Loading