diff --git a/src/backend/services/mcp/mcp-server-manager.ts b/src/backend/services/mcp/mcp-server-manager.ts index 4adf5d62..7d3aff47 100644 --- a/src/backend/services/mcp/mcp-server-manager.ts +++ b/src/backend/services/mcp/mcp-server-manager.ts @@ -223,6 +223,7 @@ export class MCPServerManager { } } for (const s of externalServers) { + if (s.builtin) continue; if (!seen.has(s.id)) { seen.add(s.id); result.push(s); diff --git a/src/shared/utils/mcp-utils.test.ts b/src/shared/utils/mcp-utils.test.ts new file mode 100644 index 00000000..1226501a --- /dev/null +++ b/src/shared/utils/mcp-utils.test.ts @@ -0,0 +1,136 @@ +import { describe, expect, it } from "vitest"; +import type { MCPServerConfig } from "../types/mcp"; +import { ensureBuiltinServerIds, getBuiltinServerIds, getConnectedOrBuiltinIds } from "./mcp-utils"; + +describe("getBuiltinServerIds", () => { + it("returns only ids of built-in servers", () => { + const servers: MCPServerConfig[] = [ + { id: "builtin-1", name: "Built-in Server 1", transport: "inMemory", builtin: true }, + { + id: "external-1", + name: "External Server", + transport: "streamableHttp", + url: "http://example.com", + }, + { id: "builtin-2", name: "Built-in Server 2", transport: "inMemory", builtin: true }, + ]; + + const result = getBuiltinServerIds(servers); + + expect(result).toEqual(["builtin-1", "builtin-2"]); + }); + + it("returns an empty array when there are no built-in servers", () => { + const servers: MCPServerConfig[] = [ + { + id: "external-1", + name: "External Server", + transport: "streamableHttp", + url: "http://example.com", + }, + ]; + + const result = getBuiltinServerIds(servers); + + expect(result).toEqual([]); + }); + + it("returns an empty array when given an empty server list", () => { + const result = getBuiltinServerIds([]); + + expect(result).toEqual([]); + }); + + it("excludes servers with missing ids", () => { + const servers: MCPServerConfig[] = [ + { name: "No ID Server", transport: "inMemory", builtin: true } as unknown as MCPServerConfig, + { + id: "builtin-1", + name: "Built-in Server 1", + transport: "inMemory", + builtin: true, + } as MCPServerConfig, + ]; + + const result = getBuiltinServerIds(servers); + + expect(result).toEqual(["builtin-1"]); + }); +}); + +describe("getConnectedOrBuiltinIds", () => { + it("includes built-in and connected servers", () => { + const servers: MCPServerConfig[] = [ + { id: "builtin-1", name: "Built-in", transport: "inMemory", builtin: true }, + { + id: "connected-1", + name: "Connected", + transport: "streamableHttp", + url: "http://example.com", + enabled: true, + }, + { + id: "disconnected-1", + name: "Disconnected", + transport: "streamableHttp", + url: "http://example.com", + enabled: false, + }, + ]; + + const result = getConnectedOrBuiltinIds(servers); + + expect(result).toEqual(new Set(["builtin-1", "connected-1"])); + }); + + it("treats servers without explicit enabled flag as connected", () => { + const servers: MCPServerConfig[] = [ + { + id: "implicit-1", + name: "Implicit", + transport: "streamableHttp", + url: "http://example.com", + }, + ]; + + const result = getConnectedOrBuiltinIds(servers); + + expect(result).toEqual(new Set(["implicit-1"])); + }); + + it("returns an empty set when given an empty list", () => { + expect(getConnectedOrBuiltinIds([])).toEqual(new Set()); + }); +}); + +describe("ensureBuiltinServerIds", () => { + it("preserves selected disabled servers while adding missing built-ins", () => { + const servers: MCPServerConfig[] = [ + { id: "builtin-1", name: "Built-in", transport: "inMemory", builtin: true }, + { + id: "disabled-1", + name: "Disabled External", + transport: "streamableHttp", + url: "http://example.com", + enabled: false, + }, + { + id: "enabled-1", + name: "Enabled External", + transport: "streamableHttp", + url: "http://example.com", + enabled: true, + }, + ]; + + expect(ensureBuiltinServerIds(["disabled-1"], servers)).toEqual(["disabled-1", "builtin-1"]); + }); + + it("avoids duplicating built-in ids that are already selected", () => { + const servers: MCPServerConfig[] = [ + { id: "builtin-1", name: "Built-in", transport: "inMemory", builtin: true }, + ]; + + expect(ensureBuiltinServerIds(["builtin-1"], servers)).toEqual(["builtin-1"]); + }); +}); diff --git a/src/shared/utils/mcp-utils.ts b/src/shared/utils/mcp-utils.ts new file mode 100644 index 00000000..fd14d07f --- /dev/null +++ b/src/shared/utils/mcp-utils.ts @@ -0,0 +1,37 @@ +import type { MCPServerConfig } from "../types/mcp"; + +/** + * Returns the IDs of built-in MCP servers from the given list. + * Used to pre-select built-in servers when creating a new custom prompt. + * The id guard defends against deserialized storage objects with missing ids. + */ +export function getBuiltinServerIds(servers: readonly MCPServerConfig[]): string[] { + return servers + .filter((s) => s.builtin) + .map((s) => s.id) + .filter((id): id is string => !!id); +} + +/** + * Returns the IDs of servers that are either built-in or currently connected (enabled). + * Used to compute which servers are selectable in the prompt editor. + */ +export function getConnectedOrBuiltinIds(servers: readonly MCPServerConfig[]): Set { + return new Set( + servers + .filter((s) => s.builtin || s.enabled !== false) + .map((s) => s.id) + .filter((id): id is string => !!id), + ); +} + +/** + * Preserves the current prompt selection while ensuring built-in servers stay selected. + * This avoids dropping temporarily disabled external servers from saved prompt configs. + */ +export function ensureBuiltinServerIds( + selectedServerIds: readonly string[] | undefined, + servers: readonly MCPServerConfig[], +): string[] { + return [...new Set([...(selectedServerIds ?? []), ...getBuiltinServerIds(servers)])]; +} diff --git a/src/ui/src/components/settings/custom-prompt/PromptForm.tsx b/src/ui/src/components/settings/custom-prompt/PromptForm.tsx index 394538fd..f253a0f5 100644 --- a/src/ui/src/components/settings/custom-prompt/PromptForm.tsx +++ b/src/ui/src/components/settings/custom-prompt/PromptForm.tsx @@ -1,4 +1,5 @@ import { zodResolver } from "@hookform/resolvers/zod"; +import { ensureBuiltinServerIds, getBuiltinServerIds } from "@shared/utils/mcp-utils"; import { ChevronLeft, ChevronRight, Copy, Trash2 } from "lucide-react"; import { useEffect, useMemo, useRef, useState } from "react"; import { useForm } from "react-hook-form"; @@ -104,20 +105,47 @@ export function PromptForm({ }); }, [mcpServers.length]); - // Auto-select all servers for existing prompts without selectedMcpServerIds - // This runs once after servers are loaded + // Initialise MCP server selection once after servers are loaded. Three cases: + // default prompt → select all servers (locked, not editable) + // new prompt → pre-select built-in server IDs only + // existing prompt → keep saved selection and ensure built-ins are always included; + // if no prior selection exists, default to all non-builtin servers // biome-ignore lint/correctness/useExhaustiveDependencies: Intentionally omitting form methods to prevent re-runs useEffect(() => { - if (serversLoaded && !isNewPrompt && !hasAutoSelectedServers.current && mcpServers.length > 0) { - const currentSelection = form.getValues("selectedMcpServerIds"); - if (!currentSelection || currentSelection.length === 0) { + if (serversLoaded && !hasAutoSelectedServers.current && mcpServers.length > 0) { + const builtinIds = getBuiltinServerIds(mcpServers); + + if (isDefault) { const allServerIds = mcpServers.map((s) => s.id).filter((id): id is string => !!id); form.setValue("selectedMcpServerIds", allServerIds, { shouldDirty: false }); + } else if (isNewPrompt) { + form.setValue("selectedMcpServerIds", builtinIds, { shouldDirty: false }); + } else { + const currentSelection = form.getValues("selectedMcpServerIds"); + if (!currentSelection || currentSelection.length === 0) { + // No prior selection: default to all non-builtin servers (regardless of connection status) + const allNonBuiltinIds = mcpServers + .filter((s) => !s.builtin) + .map((s) => s.id) + .filter((id): id is string => !!id); + form.setValue( + "selectedMcpServerIds", + ensureBuiltinServerIds(allNonBuiltinIds, mcpServers), + { shouldDirty: false }, + ); + } else { + // Existing selection: preserve it and silently add any missing built-ins + form.setValue( + "selectedMcpServerIds", + ensureBuiltinServerIds(currentSelection, mcpServers), + { shouldDirty: false }, + ); + } } hasAutoSelectedServers.current = true; } // eslint-disable-next-line react-hooks/exhaustive-deps - }, [serversLoaded, isNewPrompt, mcpServers]); + }, [serversLoaded, isDefault, isNewPrompt, mcpServers]); const handleSubmit = async (andActivate: boolean) => { const isValid = await form.trigger(); @@ -125,24 +153,31 @@ export function PromptForm({ const data = form.getValues(); - // Validate that at least one enabled non-built-in MCP server is selected (if any are available) - // Skip validation for default prompts since their server selection cannot be changed if (!isDefault) { - const availableEnabledNonBuiltinServers = mcpServers.filter( - (server) => !server.builtin && server.enabled !== false, + const nonBuiltinServers = mcpServers.filter((s) => !s.builtin); + + // Case 1: no non-builtin servers exist at all → hard block + if (nonBuiltinServers.length === 0) { + form.setError("selectedMcpServerIds", { + type: "manual", + message: "No MCP servers configured. Please add a server in the MCP settings tab.", + }); + return; + } + + // Case 2: non-builtin servers exist but none are selected → hard block + const selectedNonBuiltinIds = (data.selectedMcpServerIds ?? []).filter((id) => + nonBuiltinServers.some((s) => s.id === id), ); - if (availableEnabledNonBuiltinServers.length > 0) { - const selectedEnabledNonBuiltinServers = availableEnabledNonBuiltinServers.filter( - (server) => server.id && data.selectedMcpServerIds?.includes(server.id), - ); - if (selectedEnabledNonBuiltinServers.length === 0) { - form.setError("selectedMcpServerIds", { - type: "manual", - message: "Please select at least one enabled MCP server (excluding built-in servers)", - }); - return; - } + if (selectedNonBuiltinIds.length === 0) { + form.setError("selectedMcpServerIds", { + type: "manual", + message: "Please select at least one MCP server (excluding built-in servers).", + }); + return; } + + // Case 3: all selected non-builtins are disconnected → soft warning, allow save (handled in UI) } await onSubmit(data, andActivate); @@ -201,7 +236,7 @@ export function PromptForm({ render={({ field }) => (
- Prompt Instructions + Prompt Instructions *
+ {hasDisconnectedSelection && ( +

+ Some selected servers are disconnected. Connect them in MCP settings tab to + make their tools available. +

+ )} ); @@ -339,9 +387,10 @@ export function PromptForm({ /> )} - {serversLoaded && mcpServers.length === 0 && ( + {serversLoaded && mcpServers.every((s) => s.builtin) && (
- No MCP servers configured. Please add MCP servers in the MCP settings tab. + No external MCP servers configured. You can add additional MCP servers in the MCP + settings tab.
)}