From 17a05e451fd175d1d82a1ee96b266338f6b07005 Mon Sep 17 00:00:00 2001 From: Pascal Date: Tue, 14 Jul 2026 16:50:44 +0200 Subject: [PATCH] ui: fix mcp panel for toggle + timeout + proxy + ON/OFF state (#25631) * ui: fix MCP panel regressions after settings rework Restore the llama-server proxy switch in the Add New Server dialog. The dialog never passed useProxy/onUseProxyChange to McpServerForm, which only renders the proxy switch when the handler is provided. The flag is now wired, persisted on addServer, and reset on close. Bound the MCP connection handshake with the configured timeout. handshakeTimeoutMs was set in the server config but never consumed. The SDK timeout only covers the initialize request, not transport.start(), which can hang forever on an unreachable host. The whole handshake now races against the timeout and closes the transport on expiry so the underlying fetch or socket is aborted. Keep disabled MCP servers visible in management and chat-add UIs. Collapsing mcpDefaultServerOverrides into mcpServers[i].enabled turned the visibleMcpServers enabled filter into a visibility trap: toggling a server off outside a conversation hid it from every surface with no way to re-enable it. The filter is dropped, tools derived from health checks still skip disabled servers, and the settings page and server card render the real card instead of a skeleton for disabled servers that never receive a startup health check. * ui: clarify MCP server list semantics and add regression test Remove the visibleMcpServers getter, a filterless alias of getServers whose name invites the next refactor to put a filter back. Call sites read getServers directly, the duplicate list in the chat submenu is merged, and the misleading local variable in the sheet is renamed. A parser unit test pins the invariant: enabled is an on/off state, never a visibility filter, so disabled servers stay listed and toggleable. * ui: apply the MCP request timeout setting live to all servers The per-server requestTimeoutSeconds field was never editable in any UI and froze the global setting at server creation time, so changing the timeout in Settings was a no-op for existing servers. The field is removed from the data model and parsers, the timeout is read live from the global setting wherever a request config is built, and the misleading "Can be overridden per server" help text is dropped. A parser unit test guards against reintroducing the stored field. * ui: move the MCP request timeout into the Agentic settings section The MCP section held a single setting. The timeout is a global tool execution parameter like the other Agentic entries, so it moves there and the section is removed. Same settings key, no migration needed. * ui: remove the dead tool preview lines setting The agenticMaxToolPreviewLines setting was read into AgenticConfig and consumed by nothing: the agentic loop only uses enabled and maxTurns. Its help text described a previous architecture where only truncated previews and the final response survived the loop; tool results and intermediate turns now persist as full DB messages, so the setting had no effect at any value. Stale keys in localStorage or a server ui-config are ignored. * ui: resolve absent MCP per-chat overrides to the server enabled flag New conversations started with every MCP server off: the settings rework stopped seeding a per-conversation override list, assuming the enabled check would fall back to mcpServers[i].enabled, but it fell back to false, and the send path passed the raw stored list with no fallback at all. The per-conversation list is now sparse by contract, holding only explicit toggles, and every access point resolves a missing entry to the server's own enabled flag: the toggle display, the resolved list handed to the agentic flow, and the enabled check itself. --- .../ChatFormActionAddMcpServersSubmenu.svelte | 8 +-- .../ChatFormActionAddSheet.svelte | 8 +-- .../app/dialogs/DialogMcpServerAddNew.svelte | 7 ++- .../mcp/McpServerCard/McpServerCard.svelte | 4 +- .../app/settings/SettingsMcpServers.svelte | 15 ++++-- tools/ui/src/lib/constants/agentic.ts | 3 +- tools/ui/src/lib/constants/routes.ts | 1 - tools/ui/src/lib/constants/settings-keys.ts | 1 - .../ui/src/lib/constants/settings-registry.ts | 32 ++--------- tools/ui/src/lib/services/mcp.service.ts | 25 ++++++++- tools/ui/src/lib/stores/agentic.svelte.ts | 5 +- tools/ui/src/lib/stores/chat.svelte.ts | 2 +- .../ui/src/lib/stores/conversations.svelte.ts | 53 ++++++++----------- tools/ui/src/lib/stores/mcp.svelte.ts | 38 ++++++------- tools/ui/src/lib/stores/tools.svelte.ts | 3 +- tools/ui/src/lib/types/agentic.d.ts | 1 - tools/ui/src/lib/types/mcp.d.ts | 2 - tools/ui/src/lib/utils/mcp.ts | 5 -- .../unit/parse-mcp-server-settings.test.ts | 40 ++++++++------ 19 files changed, 126 insertions(+), 127 deletions(-) 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 f86222823d..ea4494ce5d 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 @@ -17,10 +17,10 @@ let { onMcpSettingsClick }: Props = $props(); let mcpSearchQuery = $state(''); - let allMcpServers = $derived(mcpStore.getServers()); - let mcpServers = $derived(mcpStore.visibleMcpServers); + // Every configured server is listed; `enabled` is an on/off state, + // not a visibility filter, so a disabled server stays toggleable. + let mcpServers = $derived(mcpStore.getServers()); let hasMcpServers = $derived(mcpServers.length > 0); - // let hasAnyMcpServers = $derived(allMcpServers.length > 0); let filteredMcpServers = $derived.by(() => { const query = mcpSearchQuery.toLowerCase().trim(); if (!query) return mcpServers; @@ -46,7 +46,7 @@ function handleMcpSubMenuOpen(open: boolean) { if (open) { mcpSearchQuery = ''; - mcpStore.runHealthChecksForServers(allMcpServers); + mcpStore.runHealthChecksForServers(mcpServers); } } 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 c280a01a00..6ee8eb578f 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 @@ -84,7 +84,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'; - let visibleMcpServers = $derived(mcpStore.visibleMcpServers); + let mcpServers = $derived(mcpStore.getServers());
@@ -218,13 +218,13 @@ MCP Servers - {visibleMcpServers.length} server{visibleMcpServers.length !== 1 ? 's' : ''} + {mcpServers.length} server{mcpServers.length !== 1 ? 's' : ''}
- {#each visibleMcpServers as server (server.id)} + {#each mcpServers as server (server.id)} {@const healthState = mcpStore.getHealthCheckState(server.id)} {@const hasError = healthState.status === HealthCheckStatus.ERROR} {@const displayName = mcpStore.getServerLabel(server)} @@ -267,7 +267,7 @@ {/each} - {#if visibleMcpServers.length === 0} + {#if mcpServers.length === 0}
No MCP servers configured
diff --git a/tools/ui/src/lib/components/app/dialogs/DialogMcpServerAddNew.svelte b/tools/ui/src/lib/components/app/dialogs/DialogMcpServerAddNew.svelte index ee99e6b5e3..7438b8fd65 100644 --- a/tools/ui/src/lib/components/app/dialogs/DialogMcpServerAddNew.svelte +++ b/tools/ui/src/lib/components/app/dialogs/DialogMcpServerAddNew.svelte @@ -16,6 +16,7 @@ let newServerUrl = $state(''); let newServerHeaders = $state(''); + let newServerUseProxy = $state(false); let newServerUrlError = $derived.by(() => { if (!newServerUrl.trim()) return 'URL is required'; try { @@ -35,6 +36,7 @@ if (!value) { newServerUrl = ''; newServerHeaders = ''; + newServerUseProxy = false; } open = value; onOpenChange?.(value); @@ -49,7 +51,8 @@ id: newServerId, enabled: true, url: newServerUrl.trim(), - headers: newServerHeaders.trim() || undefined + headers: newServerHeaders.trim() || undefined, + useProxy: newServerUseProxy }); conversationsStore.setMcpServerOverride(newServerId, true); @@ -74,8 +77,10 @@ (newServerUrl = v)} onHeadersChange={(v) => (newServerHeaders = v)} + onUseProxyChange={(v) => (newServerUseProxy = v)} urlError={newServerUrl ? newServerUrlError : null} id="new-server" /> diff --git a/tools/ui/src/lib/components/app/mcp/McpServerCard/McpServerCard.svelte b/tools/ui/src/lib/components/app/mcp/McpServerCard/McpServerCard.svelte index d238d1ffba..f7b6e24710 100644 --- a/tools/ui/src/lib/components/app/mcp/McpServerCard/McpServerCard.svelte +++ b/tools/ui/src/lib/components/app/mcp/McpServerCard/McpServerCard.svelte @@ -32,7 +32,9 @@ let isHealthChecking = $derived(healthState.status === HealthCheckStatus.CONNECTING); let isConnected = $derived(healthState.status === HealthCheckStatus.SUCCESS); let isError = $derived(healthState.status === HealthCheckStatus.ERROR); - let showSkeleton = $derived(isIdle || isHealthChecking); + // Disabled servers stay IDLE (no startup health check), so the body + // skeleton only applies while a check is running or expected to run. + let showSkeleton = $derived(isHealthChecking || (isIdle && server.enabled)); let errorMessage = $derived( healthState.status === HealthCheckStatus.ERROR ? healthState.message : undefined ); diff --git a/tools/ui/src/lib/components/app/settings/SettingsMcpServers.svelte b/tools/ui/src/lib/components/app/settings/SettingsMcpServers.svelte index 120e7914fa..bff2caa5b4 100644 --- a/tools/ui/src/lib/components/app/settings/SettingsMcpServers.svelte +++ b/tools/ui/src/lib/components/app/settings/SettingsMcpServers.svelte @@ -22,7 +22,9 @@ let { class: className }: Props = $props(); - let servers = $derived(mcpStore.visibleMcpServers); + // Every configured server is listed; `enabled` is an on/off state, + // not a visibility filter, so a disabled server stays toggleable. + let servers = $derived(mcpStore.getServers()); let isAddingServer = $state(false); @@ -58,9 +60,14 @@ // Each card decides for itself whether to render based on its own // health-check state, so adding a server only flashes the new card // (not every other already-loaded card) until its health check resolves. - function isServerPending(serverId: string): boolean { + // Disabled servers never receive a startup health check, so IDLE only + // counts as pending when the server is enabled; otherwise the real card + // renders and keeps the enable toggle reachable. + function isServerPending(serverId: string, enabled: boolean): boolean { const status = mcpStore.getHealthCheckState(serverId).status; - return status === HealthCheckStatus.IDLE || status === HealthCheckStatus.CONNECTING; + return ( + status === HealthCheckStatus.CONNECTING || (status === HealthCheckStatus.IDLE && enabled) + ); } @@ -109,7 +116,7 @@ style="grid-template-columns: repeat(auto-fill, minmax(min(32rem, calc(100dvw - 2rem)), 1fr));" > {#each servers as server (server.id)} - {#if isServerPending(server.id)} + {#if isServerPending(server.id, server.enabled)} {:else} = { } }, { - key: SETTINGS_KEYS.AGENTIC_MAX_TOOL_PREVIEW_LINES, - label: 'Max lines per tool preview', - help: 'Number of lines shown in tool output previews (last N lines). Only these previews and the final LLM response persist after the agentic loop completes.', - defaultValue: 25, + key: SETTINGS_KEYS.MCP_REQUEST_TIMEOUT_SECONDS, + label: 'MCP request timeout (seconds)', + help: 'Timeout for individual MCP tool calls.', + defaultValue: DEFAULT_MCP_CONFIG.requestTimeoutSeconds, type: SettingsFieldType.INPUT, section: SETTINGS_SECTION_SLUGS.AGENTIC, isPositiveInteger: true, sync: { - serverKey: SETTINGS_KEYS.AGENTIC_MAX_TOOL_PREVIEW_LINES, + serverKey: SETTINGS_KEYS.MCP_REQUEST_TIMEOUT_SECONDS, paramType: SyncableParameterType.NUMBER } } @@ -735,26 +733,6 @@ const SETTINGS_REGISTRY: Record = { } } ] - }, - [SETTINGS_SECTION_SLUGS.MCP]: { - title: SETTINGS_SECTION_TITLES.MCP, - slug: SETTINGS_SECTION_SLUGS.MCP, - icon: McpLogo, - settings: [ - { - key: SETTINGS_KEYS.MCP_REQUEST_TIMEOUT_SECONDS, - label: 'Request timeout (seconds)', - help: 'Default timeout for individual MCP tool calls. Can be overridden per server.', - defaultValue: DEFAULT_MCP_CONFIG.requestTimeoutSeconds, - type: SettingsFieldType.INPUT, - section: SETTINGS_SECTION_SLUGS.MCP, - isPositiveInteger: true, - sync: { - serverKey: SETTINGS_KEYS.MCP_REQUEST_TIMEOUT_SECONDS, - paramType: SyncableParameterType.NUMBER - } - } - ] } } as const; diff --git a/tools/ui/src/lib/services/mcp.service.ts b/tools/ui/src/lib/services/mcp.service.ts index ae98632a6f..e36faaac24 100644 --- a/tools/ui/src/lib/services/mcp.service.ts +++ b/tools/ui/src/lib/services/mcp.service.ts @@ -692,8 +692,31 @@ export class MCPService { this.createLog(MCPConnectionPhase.INITIALIZING, 'Sending initialize request...') ); + // The SDK timeout only covers the initialize request, not transport.start(), + // which can hang forever on an unreachable host (SSE endpoint wait, WebSocket + // handshake, proxied fetch). This race bounds the whole handshake and closes + // the transport on expiry so the underlying fetch or socket is aborted. + const handshakeTimeoutMs = + serverConfig.handshakeTimeoutMs ?? DEFAULT_MCP_CONFIG.connectionTimeoutMs; + try { - await client.connect(transport); + let handshakeTimer: ReturnType | undefined; + const handshakeDeadline = new Promise((_, reject) => { + handshakeTimer = setTimeout(() => { + void transport.close().catch(() => {}); + reject(new Error(`Connection timed out after ${Math.round(handshakeTimeoutMs / 1000)}s`)); + }, handshakeTimeoutMs); + }); + + try { + await Promise.race([ + client.connect(transport, { timeout: handshakeTimeoutMs }), + handshakeDeadline + ]); + } finally { + clearTimeout(handshakeTimer); + } + // Transport diagnostics are only for the initial handshake, not long-lived traffic. stopPhaseLogging(); client.onerror = runtimeErrorHandler; diff --git a/tools/ui/src/lib/stores/agentic.svelte.ts b/tools/ui/src/lib/stores/agentic.svelte.ts index 1a677602f9..a1beb4029e 100644 --- a/tools/ui/src/lib/stores/agentic.svelte.ts +++ b/tools/ui/src/lib/stores/agentic.svelte.ts @@ -280,16 +280,13 @@ class AgenticStore { getConfig(settings: SettingsConfigType, perChatOverrides?: McpServerOverride[]): AgenticConfig { const maxTurns = Number(settings.agenticMaxTurns) || DEFAULT_AGENTIC_CONFIG.maxTurns; - const maxToolPreviewLines = - Number(settings.agenticMaxToolPreviewLines) || DEFAULT_AGENTIC_CONFIG.maxToolPreviewLines; const hasTools = mcpStore.hasEnabledServers(perChatOverrides) || toolsStore.builtinTools.length > 0 || toolsStore.customTools.length > 0; return { enabled: hasTools && DEFAULT_AGENTIC_CONFIG.enabled, - maxTurns, - maxToolPreviewLines + maxTurns }; } diff --git a/tools/ui/src/lib/stores/chat.svelte.ts b/tools/ui/src/lib/stores/chat.svelte.ts index fcd07c4fd6..0a496f2c77 100644 --- a/tools/ui/src/lib/stores/chat.svelte.ts +++ b/tools/ui/src/lib/stores/chat.svelte.ts @@ -1334,7 +1334,7 @@ class ChatStore { } }; - const perChatOverrides = conversationsStore.activeConversation?.mcpServerOverrides; + const perChatOverrides = conversationsStore.getAllMcpServerOverrides(); { const agenticResult = await agenticStore.runAgenticFlow({ diff --git a/tools/ui/src/lib/stores/conversations.svelte.ts b/tools/ui/src/lib/stores/conversations.svelte.ts index ea2c11e145..c7bfd00b8c 100644 --- a/tools/ui/src/lib/stores/conversations.svelte.ts +++ b/tools/ui/src/lib/stores/conversations.svelte.ts @@ -243,9 +243,9 @@ class ConversationsStore { const conversationName = name || `Chat ${new Date().toLocaleString()}`; const conversation = await DatabaseService.createConversation(conversationName); - // New conversations inherit per-server enabled defaults directly from - // `mcpServers[i].enabled` (see #checkServerEnabled). No per-conversation - // override list needs to be seeded. + // No MCP override list is seeded: getAllMcpServerOverrides resolves + // servers without a per-conversation override to `mcpServers[i].enabled`, + // and only explicit toggles are stored on the conversation. // Inherit global thinking/reasoning defaults into the new conversation const thinkingEnabled = this.getThinkingEnabled(); @@ -601,48 +601,41 @@ class ConversationsStore { */ /** - /** - * Resolve the per-server enabled value when no active conversation exists. - * The default for new chats is the server's own `enabled` flag in `mcpServers`. + * Resolve the default enabled value for a server: its own `enabled` + * flag in `mcpServers`, so the global on/off state lives in one place. */ - #getDefaultOverrideForNoConversation(serverId: string): McpServerOverride | undefined { + #getDefaultOverride(serverId: string): McpServerOverride | undefined { const server = mcpStore.getServers().find((s) => s.id === serverId); if (!server) return undefined; return { serverId, enabled: server.enabled }; } /** - * Default overrides for new chats are derived from `mcpServers[i].enabled`, - * so the global on/off state lives in one place. - */ - #getAllDefaultOverridesForNoConversation(): McpServerOverride[] { - return mcpStore.getServers().map((s) => ({ serverId: s.id, enabled: s.enabled })); - } - - /** - * Gets MCP server override for a specific server in the active conversation. - * Falls back to `mcpServers[i].enabled` if no active conversation exists. + * Gets the effective MCP server override for a specific server. + * A per-conversation override wins when present; a server without one + * resolves to its `mcpServers[i].enabled` default. * @param serverId - The server ID to check - * @returns The override if set, undefined if no matching server + * @returns The effective override, undefined if no matching server */ getMcpServerOverride(serverId: string): McpServerOverride | undefined { - if (this.activeConversation) { - return this.activeConversation.mcpServerOverrides?.find( - (o: McpServerOverride) => o.serverId === serverId - ); - } - return this.#getDefaultOverrideForNoConversation(serverId); + const override = this.activeConversation?.mcpServerOverrides?.find( + (o: McpServerOverride) => o.serverId === serverId + ); + if (override) return override; + return this.#getDefaultOverride(serverId); } /** - * Get all MCP server overrides for the current conversation. - * When no active conversation, derives from `mcpServers[i].enabled`. + * Gets the effective override list for the current conversation: + * one entry per configured server, resolved per server. The stored + * per-conversation list is sparse and only holds explicit toggles. */ getAllMcpServerOverrides(): McpServerOverride[] { - if (this.activeConversation?.mcpServerOverrides) { - return this.activeConversation.mcpServerOverrides; - } - return this.#getAllDefaultOverridesForNoConversation(); + const overrides = this.activeConversation?.mcpServerOverrides; + return mcpStore.getServers().map((s) => { + const override = overrides?.find((o: McpServerOverride) => o.serverId === s.id); + return { serverId: s.id, enabled: override?.enabled ?? s.enabled }; + }); } /** diff --git a/tools/ui/src/lib/stores/mcp.svelte.ts b/tools/ui/src/lib/stores/mcp.svelte.ts index c752e7a366..4f05ad13ec 100644 --- a/tools/ui/src/lib/stores/mcp.svelte.ts +++ b/tools/ui/src/lib/stores/mcp.svelte.ts @@ -148,15 +148,22 @@ class MCPStore { enabled: Boolean((entry as { enabled?: unknown })?.enabled), url, name: (entry as { name?: string })?.name, - requestTimeoutSeconds: - (entry as { requestTimeoutSeconds?: number })?.requestTimeoutSeconds ?? - DEFAULT_MCP_CONFIG.requestTimeoutSeconds, headers: headers || undefined, useProxy: Boolean((entry as { useProxy?: unknown })?.useProxy) } satisfies MCPServerSettingsEntry; }); } + /** + * Request timeout in milliseconds, read live from the global setting + * so a change in Settings applies to every server immediately. + */ + #requestTimeoutMs(): number { + const seconds = + Number(config().mcpRequestTimeoutSeconds) || DEFAULT_MCP_CONFIG.requestTimeoutSeconds; + return Math.round(seconds * 1000); + } + /** * Builds server configuration from a settings entry. */ @@ -183,7 +190,7 @@ class MCPStore { url: entry.url, transport: detectMcpTransportFromUrl(entry.url), handshakeTimeoutMs: connectionTimeoutMs, - requestTimeoutMs: Math.round(entry.requestTimeoutSeconds * 1000), + requestTimeoutMs: this.#requestTimeoutMs(), headers, useProxy: entry.useProxy }; @@ -191,15 +198,15 @@ class MCPStore { /** * Checks if a server is enabled for a given chat. - * Only per-chat overrides (persisted in localStorage for new chats, - * or in IndexedDB for existing conversations) control enabled state. + * A per-chat override wins when present; a server without one resolves + * to its own `enabled` flag in `mcpServers`. */ #checkServerEnabled( server: MCPServerSettingsEntry, perChatOverrides?: McpServerOverride[] ): boolean { const override = perChatOverrides?.find((o) => o.serverId === server.id); - return override?.enabled ?? false; + return override?.enabled ?? server.enabled; } /** @@ -230,7 +237,7 @@ class MCPStore { protocolVersion: DEFAULT_MCP_CONFIG.protocolVersion, capabilities: DEFAULT_MCP_CONFIG.capabilities, clientInfo: DEFAULT_MCP_CONFIG.clientInfo, - requestTimeoutMs: Math.round(DEFAULT_MCP_CONFIG.requestTimeoutSeconds * 1000), + requestTimeoutMs: this.#requestTimeoutMs(), servers }; } @@ -500,7 +507,7 @@ class MCPStore { } addServer( - serverData: Omit & { id?: string } + serverData: Omit & { id?: string } ): MCPServerSettingsEntry { const servers = this.getServers(); const newServer: MCPServerSettingsEntry = { @@ -509,8 +516,6 @@ class MCPStore { url: serverData.url.trim(), name: serverData.name, headers: serverData.headers?.trim() || undefined, - requestTimeoutSeconds: - Number(config().mcpRequestTimeoutSeconds) || DEFAULT_MCP_CONFIG.requestTimeoutSeconds, useProxy: serverData.useProxy }; settingsStore.updateConfig(SETTINGS_KEYS.MCP_SERVERS, JSON.stringify([...servers, newServer])); @@ -551,14 +556,6 @@ class MCPStore { }); } - /** - * MCP servers selectable in chat-add UIs and the settings page, - * in the order they were added to the config. - */ - get visibleMcpServers(): MCPServerSettingsEntry[] { - return this.getServers().filter((server) => server.enabled); - } - async ensureInitialized(perChatOverrides?: McpServerOverride[]): Promise { if (!browser) { return false; @@ -1226,7 +1223,6 @@ class MCPStore { id: string; enabled: boolean; url: string; - requestTimeoutSeconds: number; headers?: string; }[], skipIfChecked = true, @@ -1317,7 +1313,7 @@ class MCPStore { logs: [] }); - const timeoutMs = Math.round(server.requestTimeoutSeconds * 1000); + const timeoutMs = this.#requestTimeoutMs(); const headers = this.parseHeaders(server.headers); try { diff --git a/tools/ui/src/lib/stores/tools.svelte.ts b/tools/ui/src/lib/stores/tools.svelte.ts index dcaab5f42b..53b6ad9ee7 100644 --- a/tools/ui/src/lib/stores/tools.svelte.ts +++ b/tools/ui/src/lib/stores/tools.svelte.ts @@ -412,7 +412,8 @@ class ToolsStore { tools: { name: string; description?: string }[]; }[] { const result: ReturnType = []; - for (const server of mcpStore.visibleMcpServers) { + for (const server of mcpStore.getServers()) { + if (!server.enabled) continue; const health = mcpStore.getHealthCheckState(server.id); if (health.status === HealthCheckStatus.SUCCESS && health.tools.length > 0) { result.push({ diff --git a/tools/ui/src/lib/types/agentic.d.ts b/tools/ui/src/lib/types/agentic.d.ts index bcec10c671..0b5a8ef6cd 100644 --- a/tools/ui/src/lib/types/agentic.d.ts +++ b/tools/ui/src/lib/types/agentic.d.ts @@ -15,7 +15,6 @@ import type { DatabaseMessage, DatabaseMessageExtra, McpServerOverride } from '. export interface AgenticConfig { enabled: boolean; maxTurns: number; - maxToolPreviewLines: number; } /** diff --git a/tools/ui/src/lib/types/mcp.d.ts b/tools/ui/src/lib/types/mcp.d.ts index dca57d3968..2666394a02 100644 --- a/tools/ui/src/lib/types/mcp.d.ts +++ b/tools/ui/src/lib/types/mcp.d.ts @@ -174,7 +174,6 @@ export interface HealthCheckParams { id: string; enabled: boolean; url: string; - requestTimeoutSeconds: number; headers?: string; useProxy?: boolean; } @@ -220,7 +219,6 @@ export interface MCPServerDisplayInfo { export type MCPServerSettingsEntry = MCPServerDisplayInfo & { enabled: boolean; - requestTimeoutSeconds: number; headers?: string; iconUrl?: string; useProxy?: boolean; diff --git a/tools/ui/src/lib/utils/mcp.ts b/tools/ui/src/lib/utils/mcp.ts index 7ce7f19fe9..b9a16ac921 100644 --- a/tools/ui/src/lib/utils/mcp.ts +++ b/tools/ui/src/lib/utils/mcp.ts @@ -9,7 +9,6 @@ import { MimeTypeText } from '$lib/enums'; import { - DEFAULT_MCP_CONFIG, MCP_SERVER_ID_PREFIX, IMAGE_FILE_EXTENSION_REGEX, CODE_FILE_EXTENSION_REGEX, @@ -64,7 +63,6 @@ export function detectMcpTransportFromUrl(url: string): MCPTransportType { /** * Parses MCP server settings from a JSON string or array. - * Preserves per-server requestTimeoutSeconds if stored, otherwise falls back to the global default. * @param rawServers - The raw servers to parse * @returns An empty array if the input is invalid. */ @@ -103,9 +101,6 @@ export function parseMcpServerSettings(rawServers: unknown): MCPServerSettingsEn enabled: Boolean((entry as { enabled?: unknown })?.enabled), url, name: (entry as { name?: string })?.name, - requestTimeoutSeconds: - (entry as { requestTimeoutSeconds?: number })?.requestTimeoutSeconds ?? - DEFAULT_MCP_CONFIG.requestTimeoutSeconds, headers: headers || undefined, useProxy: Boolean((entry as { useProxy?: unknown })?.useProxy) } satisfies MCPServerSettingsEntry; diff --git a/tools/ui/tests/unit/parse-mcp-server-settings.test.ts b/tools/ui/tests/unit/parse-mcp-server-settings.test.ts index f5e0b3a96a..8dc7dffe3a 100644 --- a/tools/ui/tests/unit/parse-mcp-server-settings.test.ts +++ b/tools/ui/tests/unit/parse-mcp-server-settings.test.ts @@ -1,6 +1,6 @@ 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'; +import { MCP_SERVER_ID_PREFIX } from '$lib/constants/mcp'; /** * Tests for the mcpServers settings parser. @@ -58,24 +58,16 @@ describe('parseMcpServerSettings', () => { 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; - + it('does not emit a per-server timeout, the request timeout is a live global setting', () => { + // A stored per-server requestTimeoutSeconds was never editable in + // any UI and froze the global setting at server creation time, + // making the Settings value a no-op for existing servers. The + // parser drops the field so the global applies live everywhere. 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 } - ]) + JSON.stringify([{ id: 'a', url: 'https://a.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); + expect(parsed[0]).not.toHaveProperty('requestTimeoutSeconds'); }); it('treats whitespace-only headers strings as undefined', () => { @@ -108,6 +100,22 @@ describe('parseMcpServerSettings', () => { expect(parsed[3]?.useProxy).toBe(true); }); + it('keeps disabled entries in the list, enabled is state and never a visibility filter', () => { + // Regression guard for issue #25625: filtering the server list on + // `enabled` hides a toggled-off server from every UI surface with + // no way to re-enable it. Any list derived from this parser must + // contain disabled entries. + const parsed = parseMcpServerSettings( + JSON.stringify([ + { id: 'on', url: 'https://on.test', enabled: true }, + { id: 'off', url: 'https://off.test', enabled: false } + ]) + ); + + expect(parsed.map((entry) => entry.id)).toEqual(['on', 'off']); + expect(parsed[1]?.enabled).toBe(false); + }); + it('preserves input order when mapping entries', () => { const source = [ { id: 'gamma', url: 'https://c.test' },