From 94875285e47516da4e1c71e7bca5ba65b82bbe59 Mon Sep 17 00:00:00 2001 From: Aleksander Grygier Date: Fri, 3 Jul 2026 12:16:29 +0200 Subject: [PATCH] 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 --- .../ChatFormActionAddMcpServersSubmenu.svelte | 2 +- .../ChatFormActionAddSheet.svelte | 10 +- .../ChatMessage/ChatMessage.svelte | 2 +- ...tMessageActionCardPermissionRequest.svelte | 40 ++-- .../app/dialogs/DialogMcpServerAddNew.svelte | 55 +++--- .../DialogMcpServerRecommendations.svelte | 180 +++++++++++++++++ .../src/lib/components/app/dialogs/index.ts | 9 + .../components/app/forms/KeyValuePairs.svelte | 23 ++- .../mcp/McpServerCard/McpServerCard.svelte | 2 +- .../McpServerCard/McpServerCardCompact.svelte | 156 +++++++++++++++ .../McpServerCardEditForm.svelte | 49 +++-- .../components/app/mcp/McpServerForm.svelte | 184 ++++++++++++++---- .../app/mcp/McpServerIdentity.svelte | 14 +- tools/ui/src/lib/components/app/mcp/index.ts | 10 + .../app/settings/SettingsMcpServers.svelte | 2 +- tools/ui/src/lib/constants/index.ts | 1 + tools/ui/src/lib/constants/mcp-form.ts | 2 + .../lib/constants/recommended-mcp-servers.ts | 35 ++++ tools/ui/src/lib/constants/settings-keys.ts | 1 + .../ui/src/lib/constants/settings-registry.ts | 10 +- tools/ui/src/lib/constants/storage.ts | 6 +- .../hooks/use-mcp-recommendations.svelte.ts | 85 ++++++++ .../ui/src/lib/services/migration.service.ts | 60 +++++- .../ui/src/lib/stores/conversations.svelte.ts | 22 +-- tools/ui/src/lib/stores/mcp.svelte.ts | 40 +++- tools/ui/src/lib/types/index.ts | 2 + tools/ui/src/lib/types/mcp.d.ts | 21 +- tools/ui/src/routes/+layout.svelte | 9 + .../components/McpServerFormWrapper.svelte | 37 ++++ .../client/mcp-server-form.svelte.test.ts | 133 +++++++++++++ tools/ui/tests/unit/headers.test.ts | 126 ++++++++++++ .../unit/parse-mcp-server-settings.test.ts | 144 ++++++++++++++ .../unit/recommended-mcp-servers.test.ts | 90 +++++++++ 33 files changed, 1411 insertions(+), 151 deletions(-) create mode 100644 tools/ui/src/lib/components/app/dialogs/DialogMcpServerRecommendations.svelte create mode 100644 tools/ui/src/lib/components/app/mcp/McpServerCard/McpServerCardCompact.svelte create mode 100644 tools/ui/src/lib/constants/recommended-mcp-servers.ts create mode 100644 tools/ui/src/lib/hooks/use-mcp-recommendations.svelte.ts create mode 100644 tools/ui/tests/client/components/McpServerFormWrapper.svelte create mode 100644 tools/ui/tests/client/mcp-server-form.svelte.test.ts create mode 100644 tools/ui/tests/unit/headers.test.ts create mode 100644 tools/ui/tests/unit/parse-mcp-server-settings.test.ts create mode 100644 tools/ui/tests/unit/recommended-mcp-servers.test.ts diff --git a/tools/ui/src/lib/components/app/chat/ChatForm/ChatFormActions/ChatFormActionAdd/ChatFormActionAddMcpServersSubmenu.svelte b/tools/ui/src/lib/components/app/chat/ChatForm/ChatFormActions/ChatFormActionAdd/ChatFormActionAddMcpServersSubmenu.svelte index dd357d6cd0..a75f45f37e 100644 --- a/tools/ui/src/lib/components/app/chat/ChatForm/ChatFormActions/ChatFormActionAdd/ChatFormActionAddMcpServersSubmenu.svelte +++ b/tools/ui/src/lib/components/app/chat/ChatForm/ChatFormActions/ChatFormActionAdd/ChatFormActionAddMcpServersSubmenu.svelte @@ -18,7 +18,7 @@ let mcpSearchQuery = $state(''); let allMcpServers = $derived(mcpStore.getServersSorted()); - let mcpServers = $derived(allMcpServers.filter((s) => s.enabled)); + let mcpServers = $derived(mcpStore.visibleMcpServers); let hasMcpServers = $derived(mcpServers.length > 0); // let hasAnyMcpServers = $derived(allMcpServers.length > 0); let filteredMcpServers = $derived.by(() => { diff --git a/tools/ui/src/lib/components/app/chat/ChatForm/ChatFormActions/ChatFormActionAdd/ChatFormActionAddSheet.svelte b/tools/ui/src/lib/components/app/chat/ChatForm/ChatFormActions/ChatFormActionAdd/ChatFormActionAddSheet.svelte index 2b708aae53..b67fb267b3 100644 --- a/tools/ui/src/lib/components/app/chat/ChatForm/ChatFormActions/ChatFormActionAdd/ChatFormActionAddSheet.svelte +++ b/tools/ui/src/lib/components/app/chat/ChatForm/ChatFormActions/ChatFormActionAdd/ChatFormActionAddSheet.svelte @@ -74,9 +74,7 @@ const sheetItemRowClass = 'flex w-full items-center justify-between gap-2 rounded-md px-3 py-2 text-left text-sm transition-colors hover:bg-accent'; - function getEnabledMcpServers() { - return mcpStore.getServersSorted().filter((s) => s.enabled); - } + let visibleMcpServers = $derived(mcpStore.visibleMcpServers);
@@ -153,13 +151,13 @@ MCP Servers - {getEnabledMcpServers().length} server{getEnabledMcpServers().length !== 1 ? 's' : ''} + {visibleMcpServers.length} server{visibleMcpServers.length !== 1 ? 's' : ''}
- {#each getEnabledMcpServers() as server (server.id)} + {#each visibleMcpServers as server (server.id)} {@const healthState = mcpStore.getHealthCheckState(server.id)} {@const hasError = healthState.status === HealthCheckStatus.ERROR} {@const displayName = mcpStore.getServerLabel(server)} @@ -202,7 +200,7 @@ {/each} - {#if getEnabledMcpServers().length === 0} + {#if visibleMcpServers.length === 0}
No MCP servers configured
diff --git a/tools/ui/src/lib/components/app/chat/ChatMessages/ChatMessage/ChatMessage.svelte b/tools/ui/src/lib/components/app/chat/ChatMessages/ChatMessage/ChatMessage.svelte index 7189ce1c76..dadcae0c49 100644 --- a/tools/ui/src/lib/components/app/chat/ChatMessages/ChatMessage/ChatMessage.svelte +++ b/tools/ui/src/lib/components/app/chat/ChatMessages/ChatMessage/ChatMessage.svelte @@ -43,7 +43,7 @@ assistantMessages: number; messageTypes: string[]; } | null>(null); - let editedContent = $state(message.content); + let editedContent = $derived(message.content); let rawEditContent = $derived.by(() => { if (message.role !== MessageRole.ASSISTANT) return undefined; diff --git a/tools/ui/src/lib/components/app/chat/ChatMessages/ChatMessageActions/ChatMessageActionCard/ChatMessageActionCardPermissionRequest.svelte b/tools/ui/src/lib/components/app/chat/ChatMessages/ChatMessageActions/ChatMessageActionCard/ChatMessageActionCardPermissionRequest.svelte index e466c84ee2..4337bb6a1e 100644 --- a/tools/ui/src/lib/components/app/chat/ChatMessages/ChatMessageActions/ChatMessageActionCard/ChatMessageActionCardPermissionRequest.svelte +++ b/tools/ui/src/lib/components/app/chat/ChatMessages/ChatMessageActions/ChatMessageActionCard/ChatMessageActionCardPermissionRequest.svelte @@ -1,8 +1,9 @@ @@ -60,29 +69,27 @@ Add New Server -
- (newServerUrl = v)} - onHeadersChange={(v) => (newServerHeaders = v)} - urlError={newServerUrl ? newServerUrlError : null} - id="new-server" - /> -
+
+
+ (newServerUrl = v)} + onHeadersChange={(v) => (newServerHeaders = v)} + urlError={newServerUrl ? newServerUrlError : null} + id="new-server" + /> +
- - + + - - + + +
diff --git a/tools/ui/src/lib/components/app/dialogs/DialogMcpServerRecommendations.svelte b/tools/ui/src/lib/components/app/dialogs/DialogMcpServerRecommendations.svelte new file mode 100644 index 0000000000..9b4489b828 --- /dev/null +++ b/tools/ui/src/lib/components/app/dialogs/DialogMcpServerRecommendations.svelte @@ -0,0 +1,180 @@ + + + + + + Do more with MCP + + Power-up your experience by adding tools, resources and more capabilities provided by MCP + servers. + + + +
+

Quickly get started with

+ + {#each RECOMMENDED_MCP_SERVERS as server (server.id)} + (selected[server.id] = enabled)} + /> + {/each} + + {#if addedServers.length > 0} + {#each addedServers as server (server.id)} + + {/each} + {/if} + + {#if showAddForm} + + (newServerUrl = v)} + onHeadersChange={(v) => (newServerHeaders = v)} + urlError={newServerUrl ? newServerUrlError : null} + id="recommendation-new-server" + /> + +
+ + + +
+ + {:else} + + + + {/if} +
+ + + + + + + + diff --git a/tools/ui/src/lib/components/app/dialogs/index.ts b/tools/ui/src/lib/components/app/dialogs/index.ts index 29136308ce..73f22c5651 100644 --- a/tools/ui/src/lib/components/app/dialogs/index.ts +++ b/tools/ui/src/lib/components/app/dialogs/index.ts @@ -18,6 +18,15 @@ */ export { default as DialogMcpServerAddNew } from './DialogMcpServerAddNew.svelte'; +/** + * **DialogMcpServerRecommendations** - Suggested MCP servers opt-in dialog + * + * Prompts the user to enable pre-defined recommended MCP servers on first launch. + * Shows one switch per suggested server and persists the choice as a per-chat + * override so the selected servers become available in conversations. + */ +export { default as DialogMcpServerRecommendations } from './DialogMcpServerRecommendations.svelte'; + /** * **DialogExportSettings** - Settings export dialog with sensitive data warning * diff --git a/tools/ui/src/lib/components/app/forms/KeyValuePairs.svelte b/tools/ui/src/lib/components/app/forms/KeyValuePairs.svelte index e0bd8d98e8..fd6e59a5b8 100644 --- a/tools/ui/src/lib/components/app/forms/KeyValuePairs.svelte +++ b/tools/ui/src/lib/components/app/forms/KeyValuePairs.svelte @@ -1,4 +1,5 @@
@@ -103,6 +123,7 @@ {#each pairs as pair, index (index)}
-
+
{#if showSkeleton} {:else if protocolVersion} diff --git a/tools/ui/src/lib/components/app/mcp/McpServerCard/McpServerCardCompact.svelte b/tools/ui/src/lib/components/app/mcp/McpServerCard/McpServerCardCompact.svelte new file mode 100644 index 0000000000..6cb3e18b65 --- /dev/null +++ b/tools/ui/src/lib/components/app/mcp/McpServerCard/McpServerCardCompact.svelte @@ -0,0 +1,156 @@ + + + +
+
+ {#if showSkeleton} + + + + + {:else} + + {/if} +
+ + +
+ + {#if isError && errorMessage} +

{errorMessage}

+ {/if} + + {#if showSkeleton} +
+ +
+ +
+ + + + +
+ {:else} + {#if description} + {#if description.lines === 2} +

+ {description.text} +

+ {:else} +

+ {description.text} +

+ {/if} + {/if} + + {#if tools.length > 0} +
+ {#each visibleTools as tool (tool.name)} + + + + {tool.name} + + + + +

+ {tool.description ?? 'No description'} +

+ + + {/each} + + {#if hiddenToolCount > 0} + + + + + {hiddenToolCount} more tools + + + + +

+ {hiddenTools.map((tool) => tool.name).join(', ')} +

+ + + {/if} +
+ {/if} + {/if} + diff --git a/tools/ui/src/lib/components/app/mcp/McpServerCard/McpServerCardEditForm.svelte b/tools/ui/src/lib/components/app/mcp/McpServerCard/McpServerCardEditForm.svelte index 6727a90006..8ed4ee8b80 100644 --- a/tools/ui/src/lib/components/app/mcp/McpServerCard/McpServerCardEditForm.svelte +++ b/tools/ui/src/lib/components/app/mcp/McpServerCard/McpServerCardEditForm.svelte @@ -1,6 +1,7 @@ -
-

Configure Server

+
+
+

Configure Server

- (editUrl = v)} - onHeadersChange={(v) => (editHeaders = v)} - onUseProxyChange={(v) => (editUseProxy = v)} - urlError={editUrl ? urlError : null} - id={serverId} - /> + (editUrl = v)} + onHeadersChange={(v) => (editHeaders = v)} + onUseProxyChange={(v) => (editUseProxy = v)} + urlError={editUrl ? urlError : null} + id={serverId} + /> -
- +
+ - + +
-
+
diff --git a/tools/ui/src/lib/components/app/mcp/McpServerForm.svelte b/tools/ui/src/lib/components/app/mcp/McpServerForm.svelte index 79738e30dd..7f05d5fef3 100644 --- a/tools/ui/src/lib/components/app/mcp/McpServerForm.svelte +++ b/tools/ui/src/lib/components/app/mcp/McpServerForm.svelte @@ -38,14 +38,87 @@ let headerPairs = $derived(parseHeadersToArray(headers)); + const AUTHORIZATION_HEADER = 'Authorization'; + const BEARER_PREFIX = 'Bearer '; + + // Heuristic: this dedicated UI only owns Authorization headers that already + // carry a Bearer scheme. Anything else (e.g. Basic, raw tokens) stays in the + // KV section so the user can still edit those values verbatim. + const matchesAuthorizationKey = (key: string): boolean => + key.trim().toLowerCase() === AUTHORIZATION_HEADER.toLowerCase(); + + const isBearerScheme = (value: string): boolean => + value.trim().toLowerCase().startsWith(BEARER_PREFIX.toLowerCase()); + + const ownedByBearerUi = (p: KeyValuePair): boolean => + matchesAuthorizationKey(p.key) && isBearerScheme(p.value); + + let hasAuthorization = $derived(headerPairs.some(ownedByBearerUi)); + + let wantsAuthorization = $state(false); + + let showAuthorization = $derived(hasAuthorization || wantsAuthorization); + + let urlInput: HTMLInputElement | null = $state(null); + let bearerInput: HTMLInputElement | null = $state(null); + + $effect(() => { + urlInput?.focus(); + }); + + $effect(() => { + if (wantsAuthorization && bearerInput) { + bearerInput.focus(); + } + }); + + let bearerToken = $derived.by(() => { + const auth = headerPairs.find(ownedByBearerUi); + if (!auth) return ''; + return auth.value.trim().slice(BEARER_PREFIX.length).trim(); + }); + + $effect(() => { + if (!headers.trim()) { + wantsAuthorization = false; + } + }); + function updateHeaderPairs(newPairs: KeyValuePair[]) { headerPairs = newPairs; onHeadersChange(serializeHeaders(newPairs)); } + + // The dedicated UI owns the Authorization slot end-to-end when the user + // engages it: any prior Authorization row (Bearer or otherwise) is replaced + // by exactly one { Authorization: "Bearer " } entry. JSON's last-key + // behavior would otherwise pick one arbitrarily, so we strip first. + function updateBearerToken(token: string) { + const filtered = headerPairs.filter((p) => !matchesAuthorizationKey(p.key)); + + const trimmed = token.trim(); + + if (trimmed) { + filtered.push({ key: AUTHORIZATION_HEADER, value: `${BEARER_PREFIX}${trimmed}` }); + } + + updateHeaderPairs(filtered); + } + + function setUseAuthorization(checked: boolean) { + wantsAuthorization = checked; + + if (!checked) { + // Only drop the entry this UI owns; a non-Bearer Authorization row + // authored in the KV section must survive a toggle off untouched. + const filtered = headerPairs.filter((p) => !ownedByBearerUi(p)); + updateHeaderPairs(filtered); + } + } -
-
+
+
@@ -57,50 +130,52 @@ value={url} oninput={(e) => onUrlChange(e.currentTarget.value)} class={urlError ? 'border-destructive' : ''} + bind:ref={urlInput} /> {#if urlError}

{urlError}

{/if} - - {#if !isWebSocket && onUseProxyChange} - - {/if}
+ + + {#if showAuthorization} +
+ updateBearerToken(e.currentTarget.value)} + class="pl-16" + bind:ref={bearerInput} + /> + + + Bearer + +
+ {/if} + !ownedByBearerUi(p))} + onPairsChange={(pairs) => { + const auth = headerPairs.find(ownedByBearerUi); + updateHeaderPairs(auth ? [...pairs, auth] : pairs); + }} keyPlaceholder="Header name" valuePlaceholder="Value" addButtonLabel="Add" @@ -108,4 +183,37 @@ sectionLabel="Custom Headers" sectionLabelOptional /> + + {#if !isWebSocket && onUseProxyChange} + + {/if}
diff --git a/tools/ui/src/lib/components/app/mcp/McpServerIdentity.svelte b/tools/ui/src/lib/components/app/mcp/McpServerIdentity.svelte index feafc5d811..3f128e02c9 100644 --- a/tools/ui/src/lib/components/app/mcp/McpServerIdentity.svelte +++ b/tools/ui/src/lib/components/app/mcp/McpServerIdentity.svelte @@ -1,6 +1,7 @@ + + + {}} + onHeadersChange={(value) => { + headersState = value; + }} + id="mcp-server-form-test" +/> + + diff --git a/tools/ui/tests/client/mcp-server-form.svelte.test.ts b/tools/ui/tests/client/mcp-server-form.svelte.test.ts new file mode 100644 index 0000000000..b4bd892347 --- /dev/null +++ b/tools/ui/tests/client/mcp-server-form.svelte.test.ts @@ -0,0 +1,133 @@ +import { describe, expect, it } from 'vitest'; +import { render } from 'vitest-browser-svelte'; +import McpServerFormWrapper from './components/McpServerFormWrapper.svelte'; + +const AUTHORIZATION_HEADER = 'Authorization'; +const BEARER_PREFIX = 'Bearer '; +const BEARER_PLACEHOLDER = 'Paste token here'; + +/** + * Client-side tests for the McpServerForm bearer UI. + * + * The dedicated UI only "owns" Authorization headers that already carry a + * Bearer scheme (heuristic check on the value). Other Authorization values + * stay in the KV section so the user can still edit them verbatim. Storage + * always goes through the same custom-headers slot, so a round-trip via this + * UI produces exactly one `Authorization: Bearer ` entry. + * + * Equivalent parser coverage lives in `tests/unit/headers.test.ts`. + */ +describe('McpServerForm - Authorization / bearer UI', () => { + function bearerInput(screen: Awaited>) { + return screen.locator.getByPlaceholder(BEARER_PLACEHOLDER); + } + + function capturedHeaders(screen: Awaited>) { + return screen.getByTestId('captured-headers'); + } + + it('mounts with the bearer input hidden when no auth header is present', async () => { + const screen = await render(McpServerFormWrapper, { headers: '' }); + + await expect.element(screen.getByRole('textbox', { name: /server url/i })).toBeVisible(); + + await expect.element(bearerInput(screen)).not.toBeInTheDocument(); + }); + + it('toggling Authorization shows the bearer input', async () => { + const screen = await render(McpServerFormWrapper, { headers: '' }); + + await screen.getByRole('switch', { name: /authorization/i }).click(); + + await expect.element(bearerInput(screen)).toBeVisible(); + }); + + it('typing a token writes the Authorization row with the Bearer prefix prepended', async () => { + const screen = await render(McpServerFormWrapper, { headers: '' }); + + await screen.getByRole('switch', { name: /authorization/i }).click(); + + const token = 'super-secret'; + await bearerInput(screen).fill(token); + + const expected = JSON.stringify({ [AUTHORIZATION_HEADER]: `${BEARER_PREFIX}${token}` }); + await expect + .element(capturedHeaders(screen)) + .toHaveAttribute('data-captured-headers', expected); + }); + + it('pre-existing Bearer header pre-fills the bearer input with the token stripped', async () => { + const existing = JSON.stringify({ + 'X-Trace-Id': 'abc', + [AUTHORIZATION_HEADER]: `${BEARER_PREFIX}preexisting` + }); + + const screen = await render(McpServerFormWrapper, { headers: existing }); + + await expect.element(bearerInput(screen)).toBeVisible(); + await expect.element(bearerInput(screen)).toHaveValue('preexisting'); + }); + + it('non-Bearer Authorization is ignored by the dedicated UI and stays in the KV section', async () => { + const existing = JSON.stringify({ [AUTHORIZATION_HEADER]: 'Basic czNjcjpwYXNz' }); + + const screen = await render(McpServerFormWrapper, { headers: existing }); + + await expect.element(bearerInput(screen)).not.toBeInTheDocument(); + + const headerKeyInput = screen.getByPlaceholder('Header name'); + await expect.element(headerKeyInput).toBeVisible(); + }); + + it('engaging the token UI replaces a non-Bearer Authorization with the Bearer scheme', async () => { + const existing = JSON.stringify({ [AUTHORIZATION_HEADER]: 'Basic old' }); + + const screen = await render(McpServerFormWrapper, { headers: existing }); + + await screen.getByRole('switch', { name: /authorization/i }).click(); + await bearerInput(screen).fill('new'); + + const expected = JSON.stringify({ [AUTHORIZATION_HEADER]: `${BEARER_PREFIX}new` }); + await expect + .element(capturedHeaders(screen)) + .toHaveAttribute('data-captured-headers', expected); + }); + + it('toggling Authorization off with no token drops the Bearer row but keeps non-Bearer schemes', async () => { + const existing = JSON.stringify({ [AUTHORIZATION_HEADER]: `${BEARER_PREFIX}xyz` }); + const screen = await render(McpServerFormWrapper, { headers: existing }); + + await screen.getByRole('switch', { name: /authorization/i }).click(); + + await expect.element(capturedHeaders(screen)).toHaveAttribute('data-captured-headers', ''); + }); + + it('toggling Authorization off when no Bearer row is present leaves headers untouched', async () => { + const existing = JSON.stringify({ [AUTHORIZATION_HEADER]: 'Basic czNjcjpwYXNz' }); + const screen = await render(McpServerFormWrapper, { headers: existing }); + + await screen.getByRole('switch', { name: /authorization/i }).click(); + await screen.getByRole('switch', { name: /authorization/i }).click(); + + await expect + .element(capturedHeaders(screen)) + .toHaveAttribute('data-captured-headers', existing); + }); + + it('clearing the bearer input drops the Authorization row', async () => { + const existing = JSON.stringify({ [AUTHORIZATION_HEADER]: `${BEARER_PREFIX}xyz` }); + const screen = await render(McpServerFormWrapper, { headers: existing }); + + await bearerInput(screen).fill(''); + + await expect.element(capturedHeaders(screen)).toHaveAttribute('data-captured-headers', ''); + }); + + it('does not surface Bearer Authorization in the KV section even when pre-existing', async () => { + const existing = JSON.stringify({ [AUTHORIZATION_HEADER]: `${BEARER_PREFIX}xyz` }); + const screen = await render(McpServerFormWrapper, { headers: existing }); + + const headerKeyInput = screen.getByPlaceholder('Header name'); + await expect.element(headerKeyInput).not.toBeInTheDocument(); + }); +}); diff --git a/tools/ui/tests/unit/headers.test.ts b/tools/ui/tests/unit/headers.test.ts new file mode 100644 index 0000000000..33619547c6 --- /dev/null +++ b/tools/ui/tests/unit/headers.test.ts @@ -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 `} + // 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); + }); +}); diff --git a/tools/ui/tests/unit/parse-mcp-server-settings.test.ts b/tools/ui/tests/unit/parse-mcp-server-settings.test.ts new file mode 100644 index 0000000000..956c677d56 --- /dev/null +++ b/tools/ui/tests/unit/parse-mcp-server-settings.test.ts @@ -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'); + }); +}); diff --git a/tools/ui/tests/unit/recommended-mcp-servers.test.ts b/tools/ui/tests/unit/recommended-mcp-servers.test.ts new file mode 100644 index 0000000000..3f6fd8f116 --- /dev/null +++ b/tools/ui/tests/unit/recommended-mcp-servers.test.ts @@ -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); + }); +});