Skip to content

Commit 17a05e4

Browse files
ui: fix mcp panel for toggle + timeout + proxy + ON/OFF state (ggml-org#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.
1 parent 7f575c3 commit 17a05e4

19 files changed

Lines changed: 126 additions & 127 deletions

tools/ui/src/lib/components/app/chat/ChatForm/ChatFormActions/ChatFormActionAdd/ChatFormActionAddMcpServersSubmenu.svelte

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,10 @@
1717
let { onMcpSettingsClick }: Props = $props();
1818
1919
let mcpSearchQuery = $state('');
20-
let allMcpServers = $derived(mcpStore.getServers());
21-
let mcpServers = $derived(mcpStore.visibleMcpServers);
20+
// Every configured server is listed; `enabled` is an on/off state,
21+
// not a visibility filter, so a disabled server stays toggleable.
22+
let mcpServers = $derived(mcpStore.getServers());
2223
let hasMcpServers = $derived(mcpServers.length > 0);
23-
// let hasAnyMcpServers = $derived(allMcpServers.length > 0);
2424
let filteredMcpServers = $derived.by(() => {
2525
const query = mcpSearchQuery.toLowerCase().trim();
2626
if (!query) return mcpServers;
@@ -46,7 +46,7 @@
4646
function handleMcpSubMenuOpen(open: boolean) {
4747
if (open) {
4848
mcpSearchQuery = '';
49-
mcpStore.runHealthChecksForServers(allMcpServers);
49+
mcpStore.runHealthChecksForServers(mcpServers);
5050
}
5151
}
5252

tools/ui/src/lib/components/app/chat/ChatForm/ChatFormActions/ChatFormActionAdd/ChatFormActionAddSheet.svelte

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@
8484
const sheetItemRowClass =
8585
'flex w-full items-center justify-between gap-2 rounded-md px-3 py-2 text-left text-sm transition-colors hover:bg-accent';
8686
87-
let visibleMcpServers = $derived(mcpStore.visibleMcpServers);
87+
let mcpServers = $derived(mcpStore.getServers());
8888
</script>
8989

9090
<div class="flex items-center gap-1 {className}">
@@ -218,13 +218,13 @@
218218
<span class="flex-1">MCP Servers</span>
219219

220220
<span class="text-xs text-muted-foreground">
221-
{visibleMcpServers.length} server{visibleMcpServers.length !== 1 ? 's' : ''}
221+
{mcpServers.length} server{mcpServers.length !== 1 ? 's' : ''}
222222
</span>
223223
</Collapsible.Trigger>
224224

225225
<Collapsible.Content>
226226
<div class="flex flex-col gap-0.5 pl-4">
227-
{#each visibleMcpServers as server (server.id)}
227+
{#each mcpServers as server (server.id)}
228228
{@const healthState = mcpStore.getHealthCheckState(server.id)}
229229
{@const hasError = healthState.status === HealthCheckStatus.ERROR}
230230
{@const displayName = mcpStore.getServerLabel(server)}
@@ -267,7 +267,7 @@
267267
</button>
268268
{/each}
269269

270-
{#if visibleMcpServers.length === 0}
270+
{#if mcpServers.length === 0}
271271
<div class="px-3 py-2 text-center text-sm text-muted-foreground">
272272
No MCP servers configured
273273
</div>

tools/ui/src/lib/components/app/dialogs/DialogMcpServerAddNew.svelte

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
1717
let newServerUrl = $state('');
1818
let newServerHeaders = $state('');
19+
let newServerUseProxy = $state(false);
1920
let newServerUrlError = $derived.by(() => {
2021
if (!newServerUrl.trim()) return 'URL is required';
2122
try {
@@ -35,6 +36,7 @@
3536
if (!value) {
3637
newServerUrl = '';
3738
newServerHeaders = '';
39+
newServerUseProxy = false;
3840
}
3941
open = value;
4042
onOpenChange?.(value);
@@ -49,7 +51,8 @@
4951
id: newServerId,
5052
enabled: true,
5153
url: newServerUrl.trim(),
52-
headers: newServerHeaders.trim() || undefined
54+
headers: newServerHeaders.trim() || undefined,
55+
useProxy: newServerUseProxy
5356
});
5457
5558
conversationsStore.setMcpServerOverride(newServerId, true);
@@ -74,8 +77,10 @@
7477
<McpServerForm
7578
url={newServerUrl}
7679
headers={newServerHeaders}
80+
useProxy={newServerUseProxy}
7781
onUrlChange={(v) => (newServerUrl = v)}
7882
onHeadersChange={(v) => (newServerHeaders = v)}
83+
onUseProxyChange={(v) => (newServerUseProxy = v)}
7984
urlError={newServerUrl ? newServerUrlError : null}
8085
id="new-server"
8186
/>

tools/ui/src/lib/components/app/mcp/McpServerCard/McpServerCard.svelte

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,9 @@
3232
let isHealthChecking = $derived(healthState.status === HealthCheckStatus.CONNECTING);
3333
let isConnected = $derived(healthState.status === HealthCheckStatus.SUCCESS);
3434
let isError = $derived(healthState.status === HealthCheckStatus.ERROR);
35-
let showSkeleton = $derived(isIdle || isHealthChecking);
35+
// Disabled servers stay IDLE (no startup health check), so the body
36+
// skeleton only applies while a check is running or expected to run.
37+
let showSkeleton = $derived(isHealthChecking || (isIdle && server.enabled));
3638
let errorMessage = $derived(
3739
healthState.status === HealthCheckStatus.ERROR ? healthState.message : undefined
3840
);

tools/ui/src/lib/components/app/settings/SettingsMcpServers.svelte

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,9 @@
2222
2323
let { class: className }: Props = $props();
2424
25-
let servers = $derived(mcpStore.visibleMcpServers);
25+
// Every configured server is listed; `enabled` is an on/off state,
26+
// not a visibility filter, so a disabled server stays toggleable.
27+
let servers = $derived(mcpStore.getServers());
2628
2729
let isAddingServer = $state(false);
2830
@@ -58,9 +60,14 @@
5860
// Each card decides for itself whether to render based on its own
5961
// health-check state, so adding a server only flashes the new card
6062
// (not every other already-loaded card) until its health check resolves.
61-
function isServerPending(serverId: string): boolean {
63+
// Disabled servers never receive a startup health check, so IDLE only
64+
// counts as pending when the server is enabled; otherwise the real card
65+
// renders and keeps the enable toggle reachable.
66+
function isServerPending(serverId: string, enabled: boolean): boolean {
6267
const status = mcpStore.getHealthCheckState(serverId).status;
63-
return status === HealthCheckStatus.IDLE || status === HealthCheckStatus.CONNECTING;
68+
return (
69+
status === HealthCheckStatus.CONNECTING || (status === HealthCheckStatus.IDLE && enabled)
70+
);
6471
}
6572
</script>
6673

@@ -109,7 +116,7 @@
109116
style="grid-template-columns: repeat(auto-fill, minmax(min(32rem, calc(100dvw - 2rem)), 1fr));"
110117
>
111118
{#each servers as server (server.id)}
112-
{#if isServerPending(server.id)}
119+
{#if isServerPending(server.id, server.enabled)}
113120
<McpServerCardSkeleton />
114121
{:else}
115122
<McpServerCard

tools/ui/src/lib/constants/agentic.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,7 @@ export const NEWLINE_SEPARATOR = '\n';
66

77
export const DEFAULT_AGENTIC_CONFIG: AgenticConfig = {
88
enabled: true,
9-
maxTurns: 100,
10-
maxToolPreviewLines: 25
9+
maxTurns: 100
1110
} as const;
1211

1312
export const REASONING_TAGS = {

tools/ui/src/lib/constants/routes.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ export const SETTINGS_SECTION_SLUGS = {
88
PENALTIES: 'penalties',
99
AGENTIC: 'agentic',
1010
DEVELOPER: 'developer',
11-
MCP: 'mcp',
1211
TOOLS: 'tools',
1312
IMPORT_EXPORT: 'import-export'
1413
} as const;

tools/ui/src/lib/constants/settings-keys.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,6 @@ export const SETTINGS_KEYS = {
5959
MCP_SERVERS: 'mcpServers',
6060
MCP_REQUEST_TIMEOUT_SECONDS: 'mcpRequestTimeoutSeconds',
6161
AGENTIC_MAX_TURNS: 'agenticMaxTurns',
62-
AGENTIC_MAX_TOOL_PREVIEW_LINES: 'agenticMaxToolPreviewLines',
6362
SHOW_TOOL_CALL_IN_PROGRESS: 'showToolCallInProgress',
6463
// Performance
6564
PRE_ENCODE_CONVERSATION: 'preEncodeConversation',

tools/ui/src/lib/constants/settings-registry.ts

Lines changed: 5 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ import type {
2424
SettingsSection
2525
} from '$lib/types';
2626
import { CLI_FLAGS, DEFAULT_MCP_CONFIG } from '$lib/constants';
27-
import McpLogo from '$lib/components/app/mcp/McpLogo.svelte';
2827
import { SETTINGS_KEYS } from './settings-keys';
2928
import { ROUTES, SETTINGS_SECTION_SLUGS } from './routes';
3029
import { TITLE_GENERATION } from './title-generation';
@@ -36,7 +35,6 @@ export const SETTINGS_SECTION_TITLES = {
3635
PENALTIES: 'Penalties',
3736
AGENTIC: 'Agentic',
3837
TOOLS: 'Tools',
39-
MCP: 'MCP',
4038
IMPORT_EXPORT: 'Import/Export',
4139
DEVELOPER: 'Developer'
4240
} as const;
@@ -635,15 +633,15 @@ const SETTINGS_REGISTRY: Record<string, SettingsSectionEntry> = {
635633
}
636634
},
637635
{
638-
key: SETTINGS_KEYS.AGENTIC_MAX_TOOL_PREVIEW_LINES,
639-
label: 'Max lines per tool preview',
640-
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.',
641-
defaultValue: 25,
636+
key: SETTINGS_KEYS.MCP_REQUEST_TIMEOUT_SECONDS,
637+
label: 'MCP request timeout (seconds)',
638+
help: 'Timeout for individual MCP tool calls.',
639+
defaultValue: DEFAULT_MCP_CONFIG.requestTimeoutSeconds,
642640
type: SettingsFieldType.INPUT,
643641
section: SETTINGS_SECTION_SLUGS.AGENTIC,
644642
isPositiveInteger: true,
645643
sync: {
646-
serverKey: SETTINGS_KEYS.AGENTIC_MAX_TOOL_PREVIEW_LINES,
644+
serverKey: SETTINGS_KEYS.MCP_REQUEST_TIMEOUT_SECONDS,
647645
paramType: SyncableParameterType.NUMBER
648646
}
649647
}
@@ -735,26 +733,6 @@ const SETTINGS_REGISTRY: Record<string, SettingsSectionEntry> = {
735733
}
736734
}
737735
]
738-
},
739-
[SETTINGS_SECTION_SLUGS.MCP]: {
740-
title: SETTINGS_SECTION_TITLES.MCP,
741-
slug: SETTINGS_SECTION_SLUGS.MCP,
742-
icon: McpLogo,
743-
settings: [
744-
{
745-
key: SETTINGS_KEYS.MCP_REQUEST_TIMEOUT_SECONDS,
746-
label: 'Request timeout (seconds)',
747-
help: 'Default timeout for individual MCP tool calls. Can be overridden per server.',
748-
defaultValue: DEFAULT_MCP_CONFIG.requestTimeoutSeconds,
749-
type: SettingsFieldType.INPUT,
750-
section: SETTINGS_SECTION_SLUGS.MCP,
751-
isPositiveInteger: true,
752-
sync: {
753-
serverKey: SETTINGS_KEYS.MCP_REQUEST_TIMEOUT_SECONDS,
754-
paramType: SyncableParameterType.NUMBER
755-
}
756-
}
757-
]
758736
}
759737
} as const;
760738

tools/ui/src/lib/services/mcp.service.ts

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -692,8 +692,31 @@ export class MCPService {
692692
this.createLog(MCPConnectionPhase.INITIALIZING, 'Sending initialize request...')
693693
);
694694

695+
// The SDK timeout only covers the initialize request, not transport.start(),
696+
// which can hang forever on an unreachable host (SSE endpoint wait, WebSocket
697+
// handshake, proxied fetch). This race bounds the whole handshake and closes
698+
// the transport on expiry so the underlying fetch or socket is aborted.
699+
const handshakeTimeoutMs =
700+
serverConfig.handshakeTimeoutMs ?? DEFAULT_MCP_CONFIG.connectionTimeoutMs;
701+
695702
try {
696-
await client.connect(transport);
703+
let handshakeTimer: ReturnType<typeof setTimeout> | undefined;
704+
const handshakeDeadline = new Promise<never>((_, reject) => {
705+
handshakeTimer = setTimeout(() => {
706+
void transport.close().catch(() => {});
707+
reject(new Error(`Connection timed out after ${Math.round(handshakeTimeoutMs / 1000)}s`));
708+
}, handshakeTimeoutMs);
709+
});
710+
711+
try {
712+
await Promise.race([
713+
client.connect(transport, { timeout: handshakeTimeoutMs }),
714+
handshakeDeadline
715+
]);
716+
} finally {
717+
clearTimeout(handshakeTimer);
718+
}
719+
697720
// Transport diagnostics are only for the initial handshake, not long-lived traffic.
698721
stopPhaseLogging();
699722
client.onerror = runtimeErrorHandler;

0 commit comments

Comments
 (0)