Skip to content

Commit 2645c36

Browse files
Copilotyaqi-lyuCopilot
authored
XS✔ ◾ ✨ Custom prompts - Default built-in MCP servers to selected (#770)
* Initial plan * feat: auto-select built-in MCP servers for new custom prompts and lock them in UI Co-authored-by: yaqi-lyu <121055451+yaqi-lyu@users.noreply.github.com> * make tools selected correctly * refactor to make it clean * perserve the selected mcp servers * decouple connecting with selected servers * Update src/ui/src/components/settings/custom-prompt/PromptForm.tsx Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * fix copilot feedback --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: yaqi-lyu <121055451+yaqi-lyu@users.noreply.github.com> Co-authored-by: Willow Lyu <iqay_lyu@hotmail.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 parent f4dd419 commit 2645c36

File tree

4 files changed

+260
-37
lines changed

4 files changed

+260
-37
lines changed

src/backend/services/mcp/mcp-server-manager.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,7 @@ export class MCPServerManager {
223223
}
224224
}
225225
for (const s of externalServers) {
226+
if (s.builtin) continue;
226227
if (!seen.has(s.id)) {
227228
seen.add(s.id);
228229
result.push(s);

src/shared/utils/mcp-utils.test.ts

Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,136 @@
1+
import { describe, expect, it } from "vitest";
2+
import type { MCPServerConfig } from "../types/mcp";
3+
import { ensureBuiltinServerIds, getBuiltinServerIds, getConnectedOrBuiltinIds } from "./mcp-utils";
4+
5+
describe("getBuiltinServerIds", () => {
6+
it("returns only ids of built-in servers", () => {
7+
const servers: MCPServerConfig[] = [
8+
{ id: "builtin-1", name: "Built-in Server 1", transport: "inMemory", builtin: true },
9+
{
10+
id: "external-1",
11+
name: "External Server",
12+
transport: "streamableHttp",
13+
url: "http://example.com",
14+
},
15+
{ id: "builtin-2", name: "Built-in Server 2", transport: "inMemory", builtin: true },
16+
];
17+
18+
const result = getBuiltinServerIds(servers);
19+
20+
expect(result).toEqual(["builtin-1", "builtin-2"]);
21+
});
22+
23+
it("returns an empty array when there are no built-in servers", () => {
24+
const servers: MCPServerConfig[] = [
25+
{
26+
id: "external-1",
27+
name: "External Server",
28+
transport: "streamableHttp",
29+
url: "http://example.com",
30+
},
31+
];
32+
33+
const result = getBuiltinServerIds(servers);
34+
35+
expect(result).toEqual([]);
36+
});
37+
38+
it("returns an empty array when given an empty server list", () => {
39+
const result = getBuiltinServerIds([]);
40+
41+
expect(result).toEqual([]);
42+
});
43+
44+
it("excludes servers with missing ids", () => {
45+
const servers: MCPServerConfig[] = [
46+
{ name: "No ID Server", transport: "inMemory", builtin: true } as unknown as MCPServerConfig,
47+
{
48+
id: "builtin-1",
49+
name: "Built-in Server 1",
50+
transport: "inMemory",
51+
builtin: true,
52+
} as MCPServerConfig,
53+
];
54+
55+
const result = getBuiltinServerIds(servers);
56+
57+
expect(result).toEqual(["builtin-1"]);
58+
});
59+
});
60+
61+
describe("getConnectedOrBuiltinIds", () => {
62+
it("includes built-in and connected servers", () => {
63+
const servers: MCPServerConfig[] = [
64+
{ id: "builtin-1", name: "Built-in", transport: "inMemory", builtin: true },
65+
{
66+
id: "connected-1",
67+
name: "Connected",
68+
transport: "streamableHttp",
69+
url: "http://example.com",
70+
enabled: true,
71+
},
72+
{
73+
id: "disconnected-1",
74+
name: "Disconnected",
75+
transport: "streamableHttp",
76+
url: "http://example.com",
77+
enabled: false,
78+
},
79+
];
80+
81+
const result = getConnectedOrBuiltinIds(servers);
82+
83+
expect(result).toEqual(new Set(["builtin-1", "connected-1"]));
84+
});
85+
86+
it("treats servers without explicit enabled flag as connected", () => {
87+
const servers: MCPServerConfig[] = [
88+
{
89+
id: "implicit-1",
90+
name: "Implicit",
91+
transport: "streamableHttp",
92+
url: "http://example.com",
93+
},
94+
];
95+
96+
const result = getConnectedOrBuiltinIds(servers);
97+
98+
expect(result).toEqual(new Set(["implicit-1"]));
99+
});
100+
101+
it("returns an empty set when given an empty list", () => {
102+
expect(getConnectedOrBuiltinIds([])).toEqual(new Set());
103+
});
104+
});
105+
106+
describe("ensureBuiltinServerIds", () => {
107+
it("preserves selected disabled servers while adding missing built-ins", () => {
108+
const servers: MCPServerConfig[] = [
109+
{ id: "builtin-1", name: "Built-in", transport: "inMemory", builtin: true },
110+
{
111+
id: "disabled-1",
112+
name: "Disabled External",
113+
transport: "streamableHttp",
114+
url: "http://example.com",
115+
enabled: false,
116+
},
117+
{
118+
id: "enabled-1",
119+
name: "Enabled External",
120+
transport: "streamableHttp",
121+
url: "http://example.com",
122+
enabled: true,
123+
},
124+
];
125+
126+
expect(ensureBuiltinServerIds(["disabled-1"], servers)).toEqual(["disabled-1", "builtin-1"]);
127+
});
128+
129+
it("avoids duplicating built-in ids that are already selected", () => {
130+
const servers: MCPServerConfig[] = [
131+
{ id: "builtin-1", name: "Built-in", transport: "inMemory", builtin: true },
132+
];
133+
134+
expect(ensureBuiltinServerIds(["builtin-1"], servers)).toEqual(["builtin-1"]);
135+
});
136+
});

src/shared/utils/mcp-utils.ts

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
import type { MCPServerConfig } from "../types/mcp";
2+
3+
/**
4+
* Returns the IDs of built-in MCP servers from the given list.
5+
* Used to pre-select built-in servers when creating a new custom prompt.
6+
* The id guard defends against deserialized storage objects with missing ids.
7+
*/
8+
export function getBuiltinServerIds(servers: readonly MCPServerConfig[]): string[] {
9+
return servers
10+
.filter((s) => s.builtin)
11+
.map((s) => s.id)
12+
.filter((id): id is string => !!id);
13+
}
14+
15+
/**
16+
* Returns the IDs of servers that are either built-in or currently connected (enabled).
17+
* Used to compute which servers are selectable in the prompt editor.
18+
*/
19+
export function getConnectedOrBuiltinIds(servers: readonly MCPServerConfig[]): Set<string> {
20+
return new Set(
21+
servers
22+
.filter((s) => s.builtin || s.enabled !== false)
23+
.map((s) => s.id)
24+
.filter((id): id is string => !!id),
25+
);
26+
}
27+
28+
/**
29+
* Preserves the current prompt selection while ensuring built-in servers stay selected.
30+
* This avoids dropping temporarily disabled external servers from saved prompt configs.
31+
*/
32+
export function ensureBuiltinServerIds(
33+
selectedServerIds: readonly string[] | undefined,
34+
servers: readonly MCPServerConfig[],
35+
): string[] {
36+
return [...new Set([...(selectedServerIds ?? []), ...getBuiltinServerIds(servers)])];
37+
}

src/ui/src/components/settings/custom-prompt/PromptForm.tsx

Lines changed: 86 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { zodResolver } from "@hookform/resolvers/zod";
2+
import { ensureBuiltinServerIds, getBuiltinServerIds } from "@shared/utils/mcp-utils";
23
import { ChevronLeft, ChevronRight, Copy, Trash2 } from "lucide-react";
34
import { useEffect, useMemo, useRef, useState } from "react";
45
import { useForm } from "react-hook-form";
@@ -104,45 +105,79 @@ export function PromptForm({
104105
});
105106
}, [mcpServers.length]);
106107

107-
// Auto-select all servers for existing prompts without selectedMcpServerIds
108-
// This runs once after servers are loaded
108+
// Initialise MCP server selection once after servers are loaded. Three cases:
109+
// default prompt → select all servers (locked, not editable)
110+
// new prompt → pre-select built-in server IDs only
111+
// existing prompt → keep saved selection and ensure built-ins are always included;
112+
// if no prior selection exists, default to all non-builtin servers
109113
// biome-ignore lint/correctness/useExhaustiveDependencies: Intentionally omitting form methods to prevent re-runs
110114
useEffect(() => {
111-
if (serversLoaded && !isNewPrompt && !hasAutoSelectedServers.current && mcpServers.length > 0) {
112-
const currentSelection = form.getValues("selectedMcpServerIds");
113-
if (!currentSelection || currentSelection.length === 0) {
115+
if (serversLoaded && !hasAutoSelectedServers.current && mcpServers.length > 0) {
116+
const builtinIds = getBuiltinServerIds(mcpServers);
117+
118+
if (isDefault) {
114119
const allServerIds = mcpServers.map((s) => s.id).filter((id): id is string => !!id);
115120
form.setValue("selectedMcpServerIds", allServerIds, { shouldDirty: false });
121+
} else if (isNewPrompt) {
122+
form.setValue("selectedMcpServerIds", builtinIds, { shouldDirty: false });
123+
} else {
124+
const currentSelection = form.getValues("selectedMcpServerIds");
125+
if (!currentSelection || currentSelection.length === 0) {
126+
// No prior selection: default to all non-builtin servers (regardless of connection status)
127+
const allNonBuiltinIds = mcpServers
128+
.filter((s) => !s.builtin)
129+
.map((s) => s.id)
130+
.filter((id): id is string => !!id);
131+
form.setValue(
132+
"selectedMcpServerIds",
133+
ensureBuiltinServerIds(allNonBuiltinIds, mcpServers),
134+
{ shouldDirty: false },
135+
);
136+
} else {
137+
// Existing selection: preserve it and silently add any missing built-ins
138+
form.setValue(
139+
"selectedMcpServerIds",
140+
ensureBuiltinServerIds(currentSelection, mcpServers),
141+
{ shouldDirty: false },
142+
);
143+
}
116144
}
117145
hasAutoSelectedServers.current = true;
118146
}
119147
// eslint-disable-next-line react-hooks/exhaustive-deps
120-
}, [serversLoaded, isNewPrompt, mcpServers]);
148+
}, [serversLoaded, isDefault, isNewPrompt, mcpServers]);
121149

122150
const handleSubmit = async (andActivate: boolean) => {
123151
const isValid = await form.trigger();
124152
if (!isValid) return;
125153

126154
const data = form.getValues();
127155

128-
// Validate that at least one enabled non-built-in MCP server is selected (if any are available)
129-
// Skip validation for default prompts since their server selection cannot be changed
130156
if (!isDefault) {
131-
const availableEnabledNonBuiltinServers = mcpServers.filter(
132-
(server) => !server.builtin && server.enabled !== false,
157+
const nonBuiltinServers = mcpServers.filter((s) => !s.builtin);
158+
159+
// Case 1: no non-builtin servers exist at all → hard block
160+
if (nonBuiltinServers.length === 0) {
161+
form.setError("selectedMcpServerIds", {
162+
type: "manual",
163+
message: "No MCP servers configured. Please add a server in the MCP settings tab.",
164+
});
165+
return;
166+
}
167+
168+
// Case 2: non-builtin servers exist but none are selected → hard block
169+
const selectedNonBuiltinIds = (data.selectedMcpServerIds ?? []).filter((id) =>
170+
nonBuiltinServers.some((s) => s.id === id),
133171
);
134-
if (availableEnabledNonBuiltinServers.length > 0) {
135-
const selectedEnabledNonBuiltinServers = availableEnabledNonBuiltinServers.filter(
136-
(server) => server.id && data.selectedMcpServerIds?.includes(server.id),
137-
);
138-
if (selectedEnabledNonBuiltinServers.length === 0) {
139-
form.setError("selectedMcpServerIds", {
140-
type: "manual",
141-
message: "Please select at least one enabled MCP server (excluding built-in servers)",
142-
});
143-
return;
144-
}
172+
if (selectedNonBuiltinIds.length === 0) {
173+
form.setError("selectedMcpServerIds", {
174+
type: "manual",
175+
message: "Please select at least one MCP server (excluding built-in servers).",
176+
});
177+
return;
145178
}
179+
180+
// Case 3: all selected non-builtins are disconnected → soft warning, allow save (handled in UI)
146181
}
147182

148183
await onSubmit(data, andActivate);
@@ -201,7 +236,7 @@ export function PromptForm({
201236
render={({ field }) => (
202237
<FormItem className="flex flex-col flex-1 min-h-0 overflow-hidden shrink-0 max-w-full">
203238
<div className="flex items-center justify-between">
204-
<FormLabel className="text-white/90 text-sm">Prompt Instructions</FormLabel>
239+
<FormLabel className="text-white/90 text-sm">Prompt Instructions *</FormLabel>
205240
<Button
206241
type="button"
207242
variant="ghost"
@@ -240,6 +275,12 @@ export function PromptForm({
240275
.filter((s) => field.value?.includes(s.id))
241276
.map((s) => s.name);
242277

278+
const hasDisconnectedSelection =
279+
!isDefault &&
280+
serversWithIds.some(
281+
(s) => !s.builtin && s.enabled === false && field.value?.includes(s.id),
282+
);
283+
243284
return (
244285
<FormItem className="shrink-0">
245286
<FormLabel>MCP Servers *</FormLabel>
@@ -262,39 +303,40 @@ export function PromptForm({
262303
aria-live="polite"
263304
>
264305
{paginatedServers.map((server) => {
265-
const isChecked = field.value?.includes(server.id) ?? false;
266-
const isDisabled = isDefault || server.enabled === false;
306+
const isBuiltin = server.builtin ?? false;
307+
const isServerDisabled = server.enabled === false;
308+
const isChecked =
309+
isDefault || isBuiltin || (field.value?.includes(server.id) ?? false);
310+
// Only lock for default prompts and built-ins; disabled servers remain toggleable
311+
const isCheckboxDisabled = isDefault || isBuiltin;
267312
const handleToggle = () => {
268313
const newValue = isChecked
269314
? (field.value || []).filter((id) => id !== server.id)
270315
: [...(field.value || []), server.id];
271316
field.onChange(newValue);
272317
};
273318
return (
274-
<div
275-
key={server.id}
276-
className={`flex items-center gap-3 p-1 rounded ${
277-
isDisabled ? "opacity-50" : ""
278-
}`}
279-
>
319+
<div key={server.id} className="flex items-center gap-3 p-1 rounded">
280320
<Checkbox
281321
id={`server-${server.id}`}
282322
checked={isChecked}
283323
onCheckedChange={handleToggle}
284-
disabled={isDisabled}
324+
disabled={isCheckboxDisabled}
285325
/>
286326
<label
287327
htmlFor={`server-${server.id}`}
288328
className={`text-sm flex-1 select-none ${
289-
isDisabled ? "cursor-not-allowed" : "cursor-pointer"
329+
isCheckboxDisabled ? "cursor-not-allowed" : "cursor-pointer"
290330
}`}
291331
>
292332
{server.name}
293-
{server.builtin && (
333+
{isBuiltin && (
294334
<span className="ml-2 text-xs text-white/50">(Built-in)</span>
295335
)}
296-
{isDisabled && (
297-
<span className="ml-2 text-xs text-yellow-500/70">(Disabled)</span>
336+
{isServerDisabled && (
337+
<span className="ml-2 text-xs text-yellow-500/70">
338+
(Disconnected)
339+
</span>
298340
)}
299341
</label>
300342
</div>
@@ -332,16 +374,23 @@ export function PromptForm({
332374
</div>
333375
)}
334376
</div>
377+
{hasDisconnectedSelection && (
378+
<p className="text-xs text-yellow-500/80 mt-1">
379+
Some selected servers are disconnected. Connect them in MCP settings tab to
380+
make their tools available.
381+
</p>
382+
)}
335383
<FormMessage />
336384
</FormItem>
337385
);
338386
}}
339387
/>
340388
)}
341389

342-
{serversLoaded && mcpServers.length === 0 && (
390+
{serversLoaded && mcpServers.every((s) => s.builtin) && (
343391
<div className="text-sm text-yellow-500/80 p-3 rounded-md border border-yellow-500/30 bg-yellow-500/10">
344-
No MCP servers configured. Please add MCP servers in the MCP settings tab.
392+
No external MCP servers configured. You can add additional MCP servers in the MCP
393+
settings tab.
345394
</div>
346395
)}
347396

0 commit comments

Comments
 (0)