diff --git a/__tests__/components/features/mcp-page/install-server-modal.test.tsx b/__tests__/components/features/mcp-page/install-server-modal.test.tsx index 8a16af8130..bb786e797a 100644 --- a/__tests__/components/features/mcp-page/install-server-modal.test.tsx +++ b/__tests__/components/features/mcp-page/install-server-modal.test.tsx @@ -250,8 +250,11 @@ describe("InstallServerModal", () => { .agent_settings_diff as { mcp_config: { mcpServers: Record }; }; + // Remote installs are now keyed by the catalog slug ("linear") rather + // than the auto-generated "shttp" fallback, so the server is + // referenceable by name in mcp_server_refs. expect(sent.mcp_config.mcpServers).toMatchObject({ - shttp: { + linear: { url: "https://mcp.linear.app/mcp", headers: { Authorization: "Bearer lin_api_secret" }, }, diff --git a/__tests__/components/features/settings/mcp-settings/mcp-server-form.validation.test.tsx b/__tests__/components/features/settings/mcp-settings/mcp-server-form.validation.test.tsx index 6b290c94b6..c0bac84a2b 100644 --- a/__tests__/components/features/settings/mcp-settings/mcp-server-form.validation.test.tsx +++ b/__tests__/components/features/settings/mcp-settings/mcp-server-form.validation.test.tsx @@ -56,6 +56,89 @@ describe("MCPServerForm validation", () => { expect(onSubmit).toHaveBeenCalledTimes(1); }); + it("includes an optional server name for sse/shttp servers", () => { + const onSubmit = vi.fn(); + + render( + , + ); + + fireEvent.change(screen.getByTestId("server-name-input"), { + target: { value: "my-search" }, + }); + fireEvent.change(screen.getByTestId("url-input"), { + target: { value: "https://api.example.com" }, + }); + + fireEvent.click(screen.getByTestId("submit-button")); + + expect(onSubmit).toHaveBeenCalledTimes(1); + expect(onSubmit.mock.calls[0][0]).toMatchObject({ + type: "sse", + name: "my-search", + url: "https://api.example.com", + }); + }); + + it("rejects an sse/shttp server name with unsafe characters", () => { + const onSubmit = vi.fn(); + + render( + , + ); + + fireEvent.change(screen.getByTestId("server-name-input"), { + target: { value: "my server" }, + }); + fireEvent.change(screen.getByTestId("url-input"), { + target: { value: "https://api.example.com" }, + }); + + fireEvent.click(screen.getByTestId("submit-button")); + + // A name with a space can't be a safe mcp_config key, so submission is + // blocked rather than persisted under a malformed key. + expect( + screen.getByText("SETTINGS$MCP_ERROR_NAME_INVALID"), + ).toBeInTheDocument(); + expect(onSubmit).not.toHaveBeenCalled(); + }); + + it("omits the name when the sse/shttp name field is left blank", () => { + const onSubmit = vi.fn(); + + render( + , + ); + + fireEvent.change(screen.getByTestId("url-input"), { + target: { value: "https://api.example.com" }, + }); + + fireEvent.click(screen.getByTestId("submit-button")); + + expect(onSubmit).toHaveBeenCalledTimes(1); + expect(onSubmit.mock.calls[0][0].name).toBeUndefined(); + }); + it("rejects duplicate URLs across sse/shttp types", () => { const onSubmit = vi.fn(); diff --git a/__tests__/utils/mcp-config.test.ts b/__tests__/utils/mcp-config.test.ts index 61d2dceec7..b978872b04 100644 --- a/__tests__/utils/mcp-config.test.ts +++ b/__tests__/utils/mcp-config.test.ts @@ -101,6 +101,98 @@ describe("toSdkMcpConfig", () => { expect(Object.keys(out!.mcpServers)).toEqual(["stdio", "stdio_1"]); }); + it("uses a user-given name as the sse/shttp dict key", () => { + const config: MCPConfig = { + sse_servers: [{ name: "my-search", url: "https://sse.example" }], + shttp_servers: [{ name: "my-docs", url: "https://shttp.example" }], + stdio_servers: [], + }; + + const out = toSdkMcpConfig(config); + + expect(Object.keys(out!.mcpServers)).toEqual(["my-search", "my-docs"]); + expect(out!.mcpServers["my-search"]).toMatchObject({ + url: "https://sse.example", + transport: "sse", + }); + }); + + it("falls back to the base name for unnamed sse/shttp entries", () => { + const config: MCPConfig = { + sse_servers: [{ name: "named", url: "https://a" }, { url: "https://b" }], + shttp_servers: [{ url: "https://c" }], + stdio_servers: [], + }; + + const out = toSdkMcpConfig(config); + + expect(Object.keys(out!.mcpServers)).toEqual(["named", "sse", "shttp"]); + }); + + it("de-dups colliding user-given sse/shttp names with a suffix", () => { + const config: MCPConfig = { + sse_servers: [ + { name: "search", url: "https://a" }, + { name: "search", url: "https://b" }, + ], + shttp_servers: [], + stdio_servers: [], + }; + + const out = toSdkMcpConfig(config); + + expect(Object.keys(out!.mcpServers)).toEqual(["search", "search_1"]); + }); + + it("round-trips a user-given sse/shttp name through parse → write", () => { + const persisted = { + mcpServers: { + "my-search": { url: "https://x", transport: "sse" }, + "my-docs": { url: "https://y" }, + }, + }; + + const parsed = parseMcpConfig(persisted); + + expect(parsed.sse_servers).toEqual([ + { name: "my-search", url: "https://x" }, + ]); + expect(parsed.shttp_servers).toEqual([ + { name: "my-docs", url: "https://y" }, + ]); + + const written = toSdkMcpConfig(parsed); + expect(Object.keys(written!.mcpServers).sort()).toEqual([ + "my-docs", + "my-search", + ]); + }); + + it("does not surface auto-generated sse/shttp keys as user names", () => { + // Keys matching the fallback pattern carry no user intent, so parsing + // must leave `name` unset — otherwise the auto key would become a + // sticky, user-facing name on the next edit. + const persisted = { + mcpServers: { + sse: { url: "https://a", transport: "sse" }, + sse_1: { url: "https://b", transport: "sse" }, + shttp: { url: "https://c" }, + shttp_2: { url: "https://d" }, + }, + }; + + const parsed = parseMcpConfig(persisted); + + expect(parsed.sse_servers).toEqual([ + { url: "https://a" }, + { url: "https://b" }, + ]); + expect(parsed.shttp_servers).toEqual([ + { url: "https://c" }, + { url: "https://d" }, + ]); + }); + it("returns null when there are no servers", () => { expect( toSdkMcpConfig({ sse_servers: [], shttp_servers: [], stdio_servers: [] }), diff --git a/src/components/features/mcp-page/install-server-modal.tsx b/src/components/features/mcp-page/install-server-modal.tsx index 4eb6183f9a..59355970b0 100644 --- a/src/components/features/mcp-page/install-server-modal.tsx +++ b/src/components/features/mcp-page/install-server-modal.tsx @@ -261,6 +261,10 @@ export function InstallServerModal({ const payload: MCPServerConfig = { id: `${template.kind}-${uuidv4()}`, type: template.kind, + // Name remote servers after the catalog slug (e.g. "github") so they + // get a referenceable mcp_config key instead of the auto-generated + // "sse"/"shttp" fallback. Stdio installs already carry serverName. + name: entry.id, url: template.url, ...(needsCredential && apiKey && { api_key: apiKey }), }; diff --git a/src/components/features/settings/mcp-settings/mcp-server-form.tsx b/src/components/features/settings/mcp-settings/mcp-server-form.tsx index 6c6d7788e3..1fec7f22b8 100644 --- a/src/components/features/settings/mcp-settings/mcp-server-form.tsx +++ b/src/components/features/settings/mcp-settings/mcp-server-form.tsx @@ -173,6 +173,14 @@ export function MCPServerForm({ const urlDupError = validateUrlUniqueness(url); if (urlDupError) return urlDupError; + // The name is optional, but when provided it becomes the mcp_config + // key (and the reference used in mcp_server_refs), so hold it to the + // same safe-identifier rule as stdio names. + const name = formData.get("name")?.toString().trim() || ""; + if (name && !/^[a-zA-Z0-9_-]+$/.test(name)) { + return t(I18nKey.SETTINGS$MCP_ERROR_NAME_INVALID); + } + // Validate timeout for SHTTP servers only if (serverType === "shttp") { const timeoutStr = formData.get("timeout")?.toString() || ""; @@ -222,12 +230,14 @@ export function MCPServerForm({ }; if (serverType === "sse" || serverType === "shttp") { + const name = formData.get("name")?.toString().trim(); const url = formData.get("url")?.toString().trim(); const apiKey = formData.get("api_key")?.toString().trim(); const timeoutStr = formData.get("timeout")?.toString().trim(); const serverConfig: MCPServerConfig = { ...baseConfig, + ...(name && { name }), url: url!, ...(apiKey && { api_key: apiKey }), }; @@ -323,6 +333,17 @@ export function MCPServerForm({ {(serverType === "sse" || serverType === "shttp") && ( <> + + ; type SdkMcpConfig = { mcpServers: Record }; +/** + * The dict key for an SSE/SHTTP server doubles as its display name once a + * user provides one. Keys that match the auto-generated fallback pattern + * (``sse``, ``shttp``, ``shttp_1``, …) emitted by {@link toSdkMcpConfig} + * carry no user intent, so we surface them as "no name" — re-serializing + * regenerates the identical fallback key and existing configs are left + * untouched. + */ +function userGivenServerName( + serverName: string, + base: "sse" | "shttp", +): string | undefined { + const autoGenerated = new RegExp(`^${base}(_\\d+)?$`); + return autoGenerated.test(serverName) ? undefined : serverName; +} + function apiKeyFromAuthorizationHeader(value: unknown): string | undefined { if (Array.isArray(value)) { return value @@ -114,11 +130,15 @@ export function parseMcpConfig(value: unknown): MCPConfig { if (apiKey) server.api_key = apiKey; migratedShttpServers.push(server); } else if (transport === "sse") { + const name = userGivenServerName(serverName, "sse"); const server: MCPSSEServer = { url }; + if (name) server.name = name; if (apiKey) server.api_key = apiKey; sseServers.push(server); } else { + const name = userGivenServerName(serverName, "shttp"); const server: MCPSHTTPServer = { url }; + if (name) server.name = name; if (apiKey) server.api_key = apiKey; if (serverConfig.timeout != null) { server.timeout = serverConfig.timeout as number; @@ -180,26 +200,30 @@ export function toSdkMcpConfig(config: MCPConfig): SdkMcpConfig | null { for (const entry of config.sse_servers) { const server: SdkMcpServerConfig = {}; + let name: string | undefined; if (typeof entry === "string") { server.url = entry; } else { + name = entry.name; server.url = entry.url; Object.assign(server, getAuthorizationHeaders(entry.api_key)); } server.transport = "sse"; - mcpServers[reserve("sse")] = server; + mcpServers[reserve(name || "sse")] = server; } for (const entry of config.shttp_servers) { const server: SdkMcpServerConfig = {}; + let name: string | undefined; if (typeof entry === "string") { server.url = entry; } else { + name = entry.name; server.url = entry.url; Object.assign(server, getAuthorizationHeaders(entry.api_key)); if (entry.timeout != null) server.timeout = entry.timeout; } - mcpServers[reserve("shttp")] = server; + mcpServers[reserve(name || "shttp")] = server; } for (const entry of config.stdio_servers) { diff --git a/src/utils/mcp-installed-servers.ts b/src/utils/mcp-installed-servers.ts index 403402b13d..d1465fcc56 100644 --- a/src/utils/mcp-installed-servers.ts +++ b/src/utils/mcp-installed-servers.ts @@ -6,6 +6,7 @@ export function flattenMcpConfig(config: MCPConfig): MCPServerConfig[] { ...config.sse_servers.map((server, index) => ({ id: `sse-${index}`, type: "sse" as const, + name: typeof server === "object" ? server.name : undefined, url: typeof server === "string" ? server : server.url, api_key: typeof server === "object" ? server.api_key : undefined, })), @@ -20,6 +21,7 @@ export function flattenMcpConfig(config: MCPConfig): MCPServerConfig[] { ...config.shttp_servers.map((server, index) => ({ id: `shttp-${index}`, type: "shttp" as const, + name: typeof server === "object" ? server.name : undefined, url: typeof server === "string" ? server : server.url, api_key: typeof server === "object" ? server.api_key : undefined, timeout: typeof server === "object" ? server.timeout : undefined, diff --git a/tests/e2e/mock-llm/mcp/mock-llm-mcp-github.spec.ts b/tests/e2e/mock-llm/mcp/mock-llm-mcp-github.spec.ts index 85b4a387ed..001cfa2cf1 100644 --- a/tests/e2e/mock-llm/mcp/mock-llm-mcp-github.spec.ts +++ b/tests/e2e/mock-llm/mcp/mock-llm-mcp-github.spec.ts @@ -152,12 +152,14 @@ test.describe("MCP GitHub server install flow", () => { const mcpConfig = settings?.agent_settings?.mcp_config; expect(mcpConfig).toBeTruthy(); - // The GitHub server should be stored as a hosted streamable HTTP server. - // The settings API redacts persisted secrets, so the raw PAT must not be - // readable after installation. + // The GitHub server should be stored as a hosted streamable HTTP server, + // keyed by the catalog slug ("github") so it is referenceable by name in + // mcp_server_refs — not the auto-generated "shttp" fallback. The settings + // API redacts persisted secrets, so the raw PAT must not be readable after + // installation. const mcpServers = mcpConfig?.mcpServers ?? mcpConfig?.shttp_servers; expect(mcpServers).toBeTruthy(); - expect(mcpServers?.shttp).toMatchObject({ + expect(mcpServers?.github).toMatchObject({ url: GITHUB_HOSTED_MCP_URL, headers: { Authorization: "", @@ -178,7 +180,7 @@ test.describe("MCP GitHub server install flow", () => { agent_settings_diff: { mcp_config: { mcpServers: { - shttp: { + github: { url: GITHUB_HOSTED_MCP_URL, headers: { Authorization: `Bearer ${FAKE_PAT}`,