ui: Add MCP Servers Opt-In for first time visitors (#25239)
* feat: ui: Add predefined recommended MCP servers to settings * feat: ui: Add MCP server recommendation dialog with custom server support * feat: Auto-focus input fields on mount and dynamic addition * feat: Add header validation to MCP server add and edit forms * feat: Persist recommended MCP server opt-in selections * test: Cover MCP configuration with tests * chore: Format & cleanup * feat: Centralize MCP server overrides to settings config and improve recommendation UI * fix: Capture index before mutation to prevent focus drift * refactor: Extract MCP_CARD_VISIBLE_TOOL_LIMIT to shared constants * refactor: Support arbitrary authorization header schemes * refactor: Consolidate MCP recommendations dismissal into existing storage key * fix: Use case-insensitive comparison for MCP server ID prefix check * refactor: Centralize MCP server visibility logic and extract recommendations hook * refactor: Cleanup
This commit is contained in:
Aleksander Grygier
@@ -0,0 +1,126 @@
|
||||
import { describe, expect, it } from 'vitest';
|
||||
import { parseHeadersToArray, serializeHeaders } from '$lib/utils/headers';
|
||||
|
||||
/**
|
||||
* Tests for the header serialization helpers used by the MCP server form
|
||||
* (custom header rows) and the new Authorization/Bearer-token flow.
|
||||
*/
|
||||
describe('parseHeadersToArray', () => {
|
||||
it('returns an empty array for empty or whitespace-only input', () => {
|
||||
expect(parseHeadersToArray('')).toEqual([]);
|
||||
expect(parseHeadersToArray(' ')).toEqual([]);
|
||||
expect(parseHeadersToArray(undefined as unknown as string)).toEqual([]);
|
||||
});
|
||||
|
||||
it('returns an empty array for invalid JSON input', () => {
|
||||
expect(parseHeadersToArray('{not-json')).toEqual([]);
|
||||
expect(parseHeadersToArray('[]')).toEqual([]);
|
||||
expect(parseHeadersToArray('"plain-string"')).toEqual([]);
|
||||
});
|
||||
|
||||
it('converts an object into ordered key/value pairs', () => {
|
||||
expect(parseHeadersToArray('{"X-Foo":"bar","Authorization":"Bearer abc"}')).toEqual([
|
||||
{ key: 'X-Foo', value: 'bar' },
|
||||
{ key: 'Authorization', value: 'Bearer abc' }
|
||||
]);
|
||||
});
|
||||
|
||||
it('stringifies non-string values', () => {
|
||||
expect(parseHeadersToArray('{"count":"42","flag":"true"}')).toEqual([
|
||||
{ key: 'count', value: '42' },
|
||||
{ key: 'flag', value: 'true' }
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('serializeHeaders', () => {
|
||||
it('returns an empty string when there are no valid pairs', () => {
|
||||
expect(serializeHeaders([])).toBe('');
|
||||
expect(serializeHeaders([{ key: '', value: 'value' }])).toBe('');
|
||||
expect(serializeHeaders([{ key: ' ', value: 'value' }])).toBe('');
|
||||
});
|
||||
|
||||
it('returns an empty string when every pair has a blank key', () => {
|
||||
expect(
|
||||
serializeHeaders([
|
||||
{ key: '', value: 'drop-me' },
|
||||
{ key: ' ', value: 'drop-me-too' },
|
||||
{ key: '\t', value: 'tab-key' }
|
||||
])
|
||||
).toBe('');
|
||||
});
|
||||
|
||||
it('drops pairs with empty keys but keeps the rest', () => {
|
||||
expect(
|
||||
serializeHeaders([
|
||||
{ key: '', value: 'drop-me' },
|
||||
{ key: 'X-Keep', value: 'ok' }
|
||||
])
|
||||
).toBe('{"X-Keep":"ok"}');
|
||||
});
|
||||
|
||||
it('trims keys before serializing', () => {
|
||||
expect(serializeHeaders([{ key: ' X-Space ', value: 'ok' }])).toBe('{"X-Space":"ok"}');
|
||||
});
|
||||
|
||||
it('preserves the input order of surviving pairs', () => {
|
||||
const serialized = serializeHeaders([
|
||||
{ key: 'X-C', value: '3' },
|
||||
{ key: 'X-A', value: '1' },
|
||||
{ key: 'X-B', value: '2' }
|
||||
]);
|
||||
|
||||
// Object key order follows insertion order in modern JS engines, so
|
||||
// the serialized JSON writes keys in our input order.
|
||||
expect(JSON.parse(serialized)).toEqual({ 'X-C': '3', 'X-A': '1', 'X-B': '2' });
|
||||
});
|
||||
});
|
||||
|
||||
describe('parseHeadersToArray / serializeHeaders roundtrip', () => {
|
||||
it('serializes back to an equal header object after a parse', () => {
|
||||
const original = JSON.stringify({
|
||||
'Content-Type': 'application/json',
|
||||
'X-Trace-Id': 'abc-123'
|
||||
});
|
||||
|
||||
const roundtrip = serializeHeaders(parseHeadersToArray(original));
|
||||
|
||||
expect(JSON.parse(roundtrip)).toEqual(JSON.parse(original));
|
||||
});
|
||||
|
||||
it('drops rows whose keys are blank after trimming during serialization', () => {
|
||||
const pairs = parseHeadersToArray('{"X-Keep":"ok","":"drop-me"}');
|
||||
|
||||
// parseHeadersToArray keeps raw key strings (the consumer is expected to
|
||||
// filter blanks, not the parser); serialization must strip them.
|
||||
expect(pairs).toEqual([
|
||||
{ key: 'X-Keep', value: 'ok' },
|
||||
{ key: '', value: 'drop-me' }
|
||||
]);
|
||||
expect(serializeHeaders(pairs)).toBe('{"X-Keep":"ok"}');
|
||||
});
|
||||
|
||||
it('preserves upstream keys untouched (does not lowercase them)', () => {
|
||||
const upperCased = '{"Authorization":"Bearer xyz"}';
|
||||
|
||||
const parsed = parseHeadersToArray(upperCased);
|
||||
|
||||
expect(parsed).toEqual([{ key: 'Authorization', value: 'Bearer xyz' }]);
|
||||
});
|
||||
|
||||
it('bearer-token write survives a re-parse when paired with regular custom headers', () => {
|
||||
// The McpServerForm bearer UI writes {Authorization: `Bearer <token>`}
|
||||
// into the same headers string as the custom KV section. The round
|
||||
// trip below mirrors the exact shape the form produces so a future
|
||||
// refactor of either code path cannot silently change the on-disk key.
|
||||
const pairs = [
|
||||
{ key: 'X-Trace-Id', value: 'abc-123' },
|
||||
{ key: 'Authorization', value: 'Bearer super-secret' }
|
||||
];
|
||||
|
||||
const serialized = serializeHeaders(pairs);
|
||||
|
||||
expect(serialized).toBe('{"X-Trace-Id":"abc-123","Authorization":"Bearer super-secret"}');
|
||||
expect(parseHeadersToArray(serialized)).toEqual(pairs);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,144 @@
|
||||
import { describe, expect, it, vi } from 'vitest';
|
||||
import { parseMcpServerSettings } from '$lib/utils/mcp';
|
||||
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.
|
||||
*/
|
||||
describe('parseMcpServerSettings', () => {
|
||||
it('returns an empty array for falsy or whitespace-only input', () => {
|
||||
expect(parseMcpServerSettings(null)).toEqual([]);
|
||||
expect(parseMcpServerSettings(undefined)).toEqual([]);
|
||||
expect(parseMcpServerSettings('')).toEqual([]);
|
||||
expect(parseMcpServerSettings(' ')).toEqual([]);
|
||||
});
|
||||
|
||||
it('returns an empty array and logs a warning for invalid JSON strings', () => {
|
||||
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
|
||||
|
||||
expect(parseMcpServerSettings('{not-json')).toEqual([]);
|
||||
expect(warn).toHaveBeenCalled();
|
||||
|
||||
warn.mockRestore();
|
||||
});
|
||||
|
||||
it('returns an empty array for valid JSON that is not an array', () => {
|
||||
expect(parseMcpServerSettings('"plain-string"')).toEqual([]);
|
||||
expect(parseMcpServerSettings('{"id":"foo"}')).toEqual([]);
|
||||
expect(parseMcpServerSettings('42')).toEqual([]);
|
||||
expect(parseMcpServerSettings('null')).toEqual([]);
|
||||
});
|
||||
|
||||
it('drops entries with no parseable id and substitutes a stable fallback', () => {
|
||||
const parsed = parseMcpServerSettings(
|
||||
JSON.stringify([{ url: 'https://a.test', enabled: true }, { url: 'https://b.test' }])
|
||||
);
|
||||
|
||||
expect(parsed).toHaveLength(2);
|
||||
expect(parsed[0]?.id).toBe(`${MCP_SERVER_ID_PREFIX}-1`);
|
||||
expect(parsed[1]?.id).toBe(`${MCP_SERVER_ID_PREFIX}-2`);
|
||||
});
|
||||
|
||||
it('reuses the first id when it is present and falls back only for missing ones', () => {
|
||||
const parsed = parseMcpServerSettings(
|
||||
JSON.stringify([
|
||||
{ id: 'custom-1', url: 'https://a.test' },
|
||||
{ url: 'https://b.test' },
|
||||
{ id: 'custom-3', url: 'https://c.test' }
|
||||
])
|
||||
);
|
||||
|
||||
expect(parsed[0]?.id).toBe('custom-1');
|
||||
expect(parsed[1]?.id).toBe(`${MCP_SERVER_ID_PREFIX}-2`);
|
||||
expect(parsed[2]?.id).toBe('custom-3');
|
||||
});
|
||||
|
||||
it('falls back to the configured default requestTimeoutSeconds only for nullish values', () => {
|
||||
const fallback = DEFAULT_MCP_CONFIG.requestTimeoutSeconds;
|
||||
|
||||
const parsed = parseMcpServerSettings(
|
||||
JSON.stringify([
|
||||
{ id: 'a', url: 'https://a.test' },
|
||||
{ id: 'b', url: 'https://b.test', requestTimeoutSeconds: undefined },
|
||||
{ id: 'c', url: 'https://c.test', requestTimeoutSeconds: 0 },
|
||||
{ id: 'd', url: 'https://d.test', requestTimeoutSeconds: 45 }
|
||||
])
|
||||
);
|
||||
|
||||
// The parser uses ?? for timeout fallback, which only triggers on
|
||||
// null/undefined. Explicit 0 is preserved at face value.
|
||||
expect(parsed[0]?.requestTimeoutSeconds).toBe(fallback);
|
||||
expect(parsed[1]?.requestTimeoutSeconds).toBe(fallback);
|
||||
expect(parsed[2]?.requestTimeoutSeconds).toBe(0);
|
||||
expect(parsed[3]?.requestTimeoutSeconds).toBe(45);
|
||||
});
|
||||
|
||||
it('treats whitespace-only headers strings as undefined', () => {
|
||||
const parsed = parseMcpServerSettings(
|
||||
JSON.stringify([
|
||||
{ id: 'a', url: 'https://a.test', headers: ' ' },
|
||||
{ id: 'b', url: 'https://b.test', headers: '{"X-Foo":"bar"}' }
|
||||
])
|
||||
);
|
||||
|
||||
// The parser trims headers and coerces empty/whitespace to undefined.
|
||||
expect(parsed[0]?.headers).toBeUndefined();
|
||||
expect(parsed[1]?.headers).toBe('{"X-Foo":"bar"}');
|
||||
});
|
||||
|
||||
it('defaults coercion for booleans (undefined -> false, true -> true)', () => {
|
||||
const parsed = parseMcpServerSettings(
|
||||
JSON.stringify([
|
||||
{ id: 'a', url: 'https://a.test' },
|
||||
{ id: 'b', url: 'https://b.test', enabled: true },
|
||||
{ id: 'c', url: 'https://c.test', enabled: false },
|
||||
{ id: 'd', url: 'https://d.test', useProxy: true }
|
||||
])
|
||||
);
|
||||
|
||||
expect(parsed[0]?.enabled).toBe(false);
|
||||
expect(parsed[1]?.enabled).toBe(true);
|
||||
expect(parsed[2]?.enabled).toBe(false);
|
||||
expect(parsed[0]?.useProxy).toBe(false);
|
||||
expect(parsed[3]?.useProxy).toBe(true);
|
||||
});
|
||||
|
||||
it('preserves input order when mapping entries', () => {
|
||||
const source = [
|
||||
{ id: 'gamma', url: 'https://c.test' },
|
||||
{ id: 'alpha', url: 'https://a.test' },
|
||||
{ id: 'beta', url: 'https://b.test' }
|
||||
];
|
||||
|
||||
const parsed = parseMcpServerSettings(JSON.stringify(source));
|
||||
|
||||
expect(parsed.map((entry) => entry.id)).toEqual(['gamma', 'alpha', 'beta']);
|
||||
});
|
||||
|
||||
it('passes non-string raw input through the JSON-equality path', () => {
|
||||
const parsed = parseMcpServerSettings([
|
||||
{ id: 'a', url: 'https://a.test' },
|
||||
{ id: 'b', url: 'https://b.test', enabled: true }
|
||||
]);
|
||||
|
||||
expect(parsed).toHaveLength(2);
|
||||
expect(parsed[0]?.id).toBe('a');
|
||||
expect(parsed[1]?.enabled).toBe(true);
|
||||
});
|
||||
|
||||
it('coerces non-string url values to an empty string rather than throwing', () => {
|
||||
const parsed = parseMcpServerSettings(
|
||||
JSON.stringify([{ id: 'a', url: 42 }, { id: 'b' }, { id: 'c', url: 'https://c.test' }])
|
||||
);
|
||||
|
||||
expect(parsed[0]?.url).toBe('');
|
||||
expect(parsed[1]?.url).toBe('');
|
||||
expect(parsed[2]?.url).toBe('https://c.test');
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,90 @@
|
||||
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