ui: Remove recommended MCP Servers + improve MCP Servers Settings UI/UX (#25535)
* fix: drop MCP recommendations auto-popup and silent preloads * feat: Add consent-driven MCP recommendations inside Add New Server dialog * refactor: Drop mcpDefaultServerOverrides for mcpServers[i].enabled * feat: Center the empty state on the MCP settings page * fix: keep existing MCP cards intact when adding a new server * fix: keep MCP cards stable when a new server is added * refactor: keep MCP server list in config insertion order * feat: shrink the recommended-MCP cards to two tools each and fit them in one row * feat: make recommended MCP cards click-to-fill and tighten copy * feat: highlight the selected MCP recommendation and stop auto-focus on dialog open * feat: derive MCP recommendation selection from the form URL * fix: make recommendation MCP cards fully non-focusable * fix: redirect focus from first card to the URL input on consent * chore: Formatting * refactor: Remove Recommended MCP Servers completely * fix: Preserve legacy mcpDefaultServerOverrides key after merge migration for downgrade compatibility
This commit is contained in:
Aleksander Grygier
@@ -0,0 +1,158 @@
|
||||
import { afterEach, beforeAll, beforeEach, describe, expect, it } from 'vitest';
|
||||
import { STORAGE_APP_NAME, CONFIG_LOCALSTORAGE_KEY } from '$lib/constants';
|
||||
|
||||
// node env unit project has no DOM, install a minimal localStorage backed by a Map
|
||||
beforeAll(() => {
|
||||
const store = new Map<string, string>();
|
||||
const polyfill: Storage = {
|
||||
get length() {
|
||||
return store.size;
|
||||
},
|
||||
clear: () => store.clear(),
|
||||
getItem: (k) => (store.has(k) ? store.get(k)! : null),
|
||||
key: (i) => Array.from(store.keys())[i] ?? null,
|
||||
removeItem: (k) => {
|
||||
store.delete(k);
|
||||
},
|
||||
setItem: (k, v) => {
|
||||
store.set(k, String(v));
|
||||
}
|
||||
};
|
||||
(globalThis as unknown as { localStorage: Storage }).localStorage = polyfill;
|
||||
});
|
||||
|
||||
/**
|
||||
* Migration `mcp-default-overrides-merge-v1` folds the values of the parallel
|
||||
* `mcpDefaultServerOverrides` config entry onto `mcpServers[i].enabled` (the
|
||||
* single source of truth for new-chat defaults). The legacy key is kept on
|
||||
* disk for downgrade compatibility.
|
||||
*/
|
||||
describe('mcp-default-overrides-merge-v1 migration', () => {
|
||||
const MIGRATION_STATE_KEY = `${STORAGE_APP_NAME}.migration-state`;
|
||||
const MCP_DEFAULT_OVERRIDES_KEY = `${STORAGE_APP_NAME}.mcpDefaultServerOverrides`;
|
||||
|
||||
beforeEach(async () => {
|
||||
localStorage.clear();
|
||||
// Reset the migration run counter so `runAllMigrations` is guaranteed to execute.
|
||||
await import('$lib/services/migration.service').then((mod) =>
|
||||
mod.MigrationService.resetState()
|
||||
);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
localStorage.clear();
|
||||
});
|
||||
|
||||
async function runMigrations() {
|
||||
const { MigrationService } = await import('$lib/services/migration.service');
|
||||
await MigrationService.runAllMigrations();
|
||||
}
|
||||
|
||||
function readConfig(): Record<string, unknown> {
|
||||
const raw = localStorage.getItem(CONFIG_LOCALSTORAGE_KEY);
|
||||
|
||||
return raw ? (JSON.parse(raw) as Record<string, unknown>) : {};
|
||||
}
|
||||
|
||||
function writeConfig(config: Record<string, unknown>) {
|
||||
localStorage.setItem(CONFIG_LOCALSTORAGE_KEY, JSON.stringify(config));
|
||||
}
|
||||
|
||||
it('applies matching overrides onto mcpServers[i].enabled and preserves the legacy key', async () => {
|
||||
writeConfig({
|
||||
mcpServers: JSON.stringify([
|
||||
{ id: 'exa', enabled: false, url: 'https://mcp.exa.ai/mcp' },
|
||||
{ id: 'hf', enabled: false, url: 'https://huggingface.co/mcp' }
|
||||
]),
|
||||
[MCP_DEFAULT_OVERRIDES_KEY]: JSON.stringify([
|
||||
{ serverId: 'exa', enabled: true },
|
||||
{ serverId: 'hf', enabled: false }
|
||||
])
|
||||
});
|
||||
|
||||
await runMigrations();
|
||||
|
||||
const after = readConfig();
|
||||
const servers = JSON.parse(after.mcpServers as string) as Array<{
|
||||
id: string;
|
||||
enabled: boolean;
|
||||
}>;
|
||||
|
||||
expect(servers.find((s) => s.id === 'exa')?.enabled).toBe(true);
|
||||
expect(servers.find((s) => s.id === 'hf')?.enabled).toBe(false);
|
||||
expect(MCP_DEFAULT_OVERRIDES_KEY in after).toBe(true);
|
||||
});
|
||||
|
||||
it('skips override ids that do not match any configured server', async () => {
|
||||
writeConfig({
|
||||
mcpServers: JSON.stringify([{ id: 'exa', enabled: false, url: 'https://mcp.exa.ai/mcp' }]),
|
||||
[MCP_DEFAULT_OVERRIDES_KEY]: JSON.stringify([
|
||||
{ serverId: 'orphan', enabled: true },
|
||||
{ serverId: 'exa', enabled: true }
|
||||
])
|
||||
});
|
||||
|
||||
await runMigrations();
|
||||
|
||||
const after = readConfig();
|
||||
const servers = JSON.parse(after.mcpServers as string) as Array<{
|
||||
id: string;
|
||||
enabled: boolean;
|
||||
}>;
|
||||
|
||||
expect(servers).toHaveLength(1);
|
||||
expect(servers[0].enabled).toBe(true);
|
||||
expect(MCP_DEFAULT_OVERRIDES_KEY in after).toBe(true);
|
||||
});
|
||||
|
||||
it('is a no-op when there are no legacy overrides', async () => {
|
||||
writeConfig({
|
||||
mcpServers: JSON.stringify([{ id: 'exa', enabled: true, url: 'https://mcp.exa.ai/mcp' }])
|
||||
});
|
||||
|
||||
await runMigrations();
|
||||
|
||||
const after = readConfig();
|
||||
const servers = JSON.parse(after.mcpServers as string) as Array<{
|
||||
id: string;
|
||||
enabled: boolean;
|
||||
}>;
|
||||
|
||||
expect(servers[0].enabled).toBe(true);
|
||||
expect(MCP_DEFAULT_OVERRIDES_KEY in after).toBe(false);
|
||||
});
|
||||
|
||||
it('does not rewrite mcpServers when override.enabled already matches', async () => {
|
||||
const originalServers = JSON.stringify([
|
||||
{ id: 'exa', enabled: true, url: 'https://mcp.exa.ai/mcp' }
|
||||
]);
|
||||
|
||||
writeConfig({
|
||||
mcpServers: originalServers,
|
||||
[MCP_DEFAULT_OVERRIDES_KEY]: JSON.stringify([{ serverId: 'exa', enabled: true }])
|
||||
});
|
||||
|
||||
await runMigrations();
|
||||
|
||||
const after = readConfig();
|
||||
expect(after.mcpServers).toBe(originalServers);
|
||||
expect(MCP_DEFAULT_OVERRIDES_KEY in after).toBe(true);
|
||||
});
|
||||
|
||||
it('records itself as completed so subsequent loads do not re-run', async () => {
|
||||
writeConfig({
|
||||
mcpServers: JSON.stringify([{ id: 'exa', enabled: false, url: 'https://mcp.exa.ai/mcp' }]),
|
||||
[MCP_DEFAULT_OVERRIDES_KEY]: JSON.stringify([{ serverId: 'exa', enabled: true }])
|
||||
});
|
||||
|
||||
const { MigrationService } = await import('$lib/services/migration.service');
|
||||
|
||||
await MigrationService.runAllMigrations();
|
||||
|
||||
const stateRaw = localStorage.getItem(MIGRATION_STATE_KEY);
|
||||
expect(stateRaw).not.toBeNull();
|
||||
const state = JSON.parse(stateRaw!) as { completed: string[]; failed: string[] };
|
||||
expect(state.completed).toContain('mcp-default-overrides-merge-v1');
|
||||
expect(state.failed).not.toContain('mcp-default-overrides-merge-v1');
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,19 @@
|
||||
import { describe, expect, it } from 'vitest';
|
||||
import { SETTINGS_KEYS } from '$lib/constants/settings-keys';
|
||||
|
||||
/**
|
||||
* Default-value policy for the `MCP_SERVERS` setting.
|
||||
*
|
||||
* Earlier versions of the UI preloaded a hard-coded list of suggested
|
||||
* MCP servers into this setting on first install. That caused silent
|
||||
* third-party HTTP requests at app load (see issue #25509) and a popup
|
||||
* "recommendation" dialog (see issue #25274). New users must now opt
|
||||
* in explicitly when adding a server, so the default is an empty list.
|
||||
*/
|
||||
describe('MCP_SERVERS default value', () => {
|
||||
it('does not preload any servers in the MCP_SERVERS setting default', async () => {
|
||||
const { SETTING_CONFIG_DEFAULT } = await import('$lib/constants/settings-registry');
|
||||
|
||||
expect(SETTING_CONFIG_DEFAULT[SETTINGS_KEYS.MCP_SERVERS]).toBe('[]');
|
||||
}, 15000);
|
||||
});
|
||||
@@ -5,11 +5,10 @@ import { DEFAULT_MCP_CONFIG, MCP_SERVER_ID_PREFIX } from '$lib/constants/mcp';
|
||||
/**
|
||||
* Tests for the mcpServers settings parser.
|
||||
*
|
||||
* The branch seeds the MCP servers setting with a default value of
|
||||
* `JSON.stringify(RECOMMENDED_MCP_SERVERS)`, so the parser has to be
|
||||
* resilient to anything that may live in the user's localStorage: malformed
|
||||
* JSON, wrong shapes, missing fields, falsy-but-not-zero numbers, and entry
|
||||
* arrays that have been mutated by the user via the settings form.
|
||||
* The parser has to be resilient to anything that may live in the
|
||||
* user's localStorage: malformed JSON, wrong shapes, missing fields,
|
||||
* falsy-but-not-zero numbers, and entry arrays that have been mutated
|
||||
* by the user via the settings form.
|
||||
*/
|
||||
describe('parseMcpServerSettings', () => {
|
||||
it('returns an empty array for falsy or whitespace-only input', () => {
|
||||
|
||||
@@ -1,90 +0,0 @@
|
||||
import { describe, expect, it } from 'vitest';
|
||||
import {
|
||||
RECOMMENDED_MCP_SERVER_IDS,
|
||||
RECOMMENDED_MCP_SERVERS
|
||||
} from '$lib/constants/recommended-mcp-servers';
|
||||
import { parseMcpServerSettings } from '$lib/utils/mcp';
|
||||
import { DEFAULT_MCP_CONFIG, MCP_SERVER_ID_PREFIX } from '$lib/constants/mcp';
|
||||
|
||||
/**
|
||||
* Tests for the predefined recommended MCP servers.
|
||||
*
|
||||
* These are surfaced to first-time users via
|
||||
* DialogMcpServerRecommendations and used as the default value of the MCP
|
||||
* servers setting, so a regression that breaks the round-trip through the
|
||||
* settings parser would silently break onboarding for new users.
|
||||
*/
|
||||
describe('RECOMMENDED_MCP_SERVERS', () => {
|
||||
it('lists at least one entry and uses stable, unique ids', () => {
|
||||
expect(RECOMMENDED_MCP_SERVERS.length).toBeGreaterThan(0);
|
||||
|
||||
const ids = RECOMMENDED_MCP_SERVERS.map((server) => server.id);
|
||||
expect(new Set(ids).size).toBe(ids.length);
|
||||
|
||||
for (const id of ids) {
|
||||
expect(id).toMatch(/^[a-z0-9-]+$/);
|
||||
expect(id.toLowerCase()).not.toContain(MCP_SERVER_ID_PREFIX.toLowerCase());
|
||||
}
|
||||
});
|
||||
|
||||
it('requires a name, description and url for every entry', () => {
|
||||
for (const server of RECOMMENDED_MCP_SERVERS) {
|
||||
expect(server.name?.trim().length ?? 0).toBeGreaterThan(0);
|
||||
expect(server.description.trim().length).toBeGreaterThan(0);
|
||||
expect(server.url.trim().length).toBeGreaterThan(0);
|
||||
expect(() => new URL(server.url)).not.toThrow();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('RECOMMENDED_MCP_SERVER_IDS', () => {
|
||||
it('matches the ids declared in RECOMMENDED_MCP_SERVERS', () => {
|
||||
expect(RECOMMENDED_MCP_SERVER_IDS.size).toBe(RECOMMENDED_MCP_SERVERS.length);
|
||||
|
||||
for (const server of RECOMMENDED_MCP_SERVERS) {
|
||||
expect(RECOMMENDED_MCP_SERVER_IDS.has(server.id)).toBe(true);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('recommended-mcp-servers default value', () => {
|
||||
it('round-trips cleanly through parseMcpServerSettings', () => {
|
||||
const serialized = JSON.stringify(RECOMMENDED_MCP_SERVERS);
|
||||
const parsed = parseMcpServerSettings(serialized);
|
||||
|
||||
expect(parsed).toHaveLength(RECOMMENDED_MCP_SERVERS.length);
|
||||
|
||||
for (let index = 0; index < RECOMMENDED_MCP_SERVERS.length; index++) {
|
||||
const source = RECOMMENDED_MCP_SERVERS[index];
|
||||
const entry = parsed[index];
|
||||
|
||||
expect(entry).toBeDefined();
|
||||
expect(entry?.id).toBe(source.id);
|
||||
expect(entry?.url).toBe(source.url);
|
||||
expect(entry?.enabled).toBe(source.enabled);
|
||||
expect(entry?.requestTimeoutSeconds).toBe(source.requestTimeoutSeconds);
|
||||
expect(entry?.name).toBe(source.name);
|
||||
|
||||
// Headers and useProxy are not set on recommended servers; the
|
||||
// parser must fall back to the inactive defaults rather than
|
||||
// surfacing undefined-boundary states.
|
||||
expect(entry?.headers).toBeUndefined();
|
||||
expect(entry?.useProxy).toBe(false);
|
||||
}
|
||||
});
|
||||
|
||||
it('uses the global default timeout when one is not specified on an entry', () => {
|
||||
const sourceOnlyRequired = {
|
||||
id: 'roundtrip-only',
|
||||
name: 'Only required fields',
|
||||
url: 'https://example.test/mcp',
|
||||
description: 'Smoke entry for parser roundtrip with default timeout.',
|
||||
enabled: true
|
||||
};
|
||||
|
||||
const parsed = parseMcpServerSettings(JSON.stringify([sourceOnlyRequired]));
|
||||
const entry = parsed[0];
|
||||
|
||||
expect(entry?.requestTimeoutSeconds).toBe(DEFAULT_MCP_CONFIG.requestTimeoutSeconds);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user