mirror of
https://github.com/OpenHands/OpenHands.git
synced 2026-10-07 14:17:58 +08:00
[AgentProfile][canvas] Give SSE/shttp MCP servers referenceable names (custom editor + marketplace) (#1386)
* feat(mcp): give SSE/shttp MCP servers a user-given name
SSE/shttp servers were serialized under auto-generated dict keys
("sse", "shttp", "shttp_1"), making them unreferenceable by name in
AgentProfile mcp_server_refs. Surface an optional "Server name" field
so they get a stable, user-meaningful key — consistent with stdio.
- Add name? to MCPSSEServer / MCPSHTTPServer
- toSdkMcpConfig: reserve(entry.name || "sse"/"shttp") so the name
becomes the dict key; reserve() still de-dups collisions
- parseMcpConfig: round-trip user-given names, but treat auto-generated
keys as nameless so existing configs re-serialize unchanged
- Plumb name through the add/update mutations and flattenMcpConfig
- Add optional "Server name" input to the SSE/shttp editor form
Part of #3726 (AgentProfile epic #3713).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* feat(mcp): name marketplace remote installs after the catalog slug
The marketplace install path builds its own payload, separate from the
custom editor. Stdio entries already carry serverName, but remote
(sse/shttp) installs set no name — so GitHub, Linear, etc. landed under
the auto-generated "sse"/"shttp" key and were unreferenceable in
mcp_server_refs, the same gap the custom-editor fix addressed.
Set name: entry.id on the remote-install payload so e.g. GitHub keys as
"github". reserve() still de-dups repeat installs.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* test(e2e): expect GitHub MCP install under the "github" key
The marketplace remote-install path now names servers after the catalog
slug, so a GitHub install persists under mcpServers.github (referenceable
in mcp_server_refs) rather than the auto-generated "shttp" fallback.
Update the install/delete e2e assertions accordingly.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(mcp): validate the optional SSE/shttp server name as a safe key
The Server name we added for SSE/shttp becomes the mcp_config dict key
(and the mcp_server_refs reference), but the form only validated stdio
names. An sse/shttp name with spaces/special chars would produce a
malformed key. Apply the same ^[a-zA-Z0-9_-]+$ rule stdio uses, while
keeping the field optional.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Debug Agent <simon@openhands.dev>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
committed by
GitHub
co-authored by
Claude Opus 4.8
Debug Agent
parent
809c17a5fd
commit
944679d9fd
@@ -250,8 +250,11 @@ describe("InstallServerModal", () => {
|
||||
.agent_settings_diff as {
|
||||
mcp_config: { mcpServers: Record<string, unknown> };
|
||||
};
|
||||
// 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" },
|
||||
},
|
||||
|
||||
+83
@@ -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(
|
||||
<MCPServerForm
|
||||
mode="add"
|
||||
server={{ id: "tmp", type: "sse" }}
|
||||
existingServers={[]}
|
||||
onSubmit={onSubmit}
|
||||
onCancel={noop}
|
||||
/>,
|
||||
);
|
||||
|
||||
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(
|
||||
<MCPServerForm
|
||||
mode="add"
|
||||
server={{ id: "tmp", type: "sse" }}
|
||||
existingServers={[]}
|
||||
onSubmit={onSubmit}
|
||||
onCancel={noop}
|
||||
/>,
|
||||
);
|
||||
|
||||
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(
|
||||
<MCPServerForm
|
||||
mode="add"
|
||||
server={{ id: "tmp", type: "shttp" }}
|
||||
existingServers={[]}
|
||||
onSubmit={onSubmit}
|
||||
onCancel={noop}
|
||||
/>,
|
||||
);
|
||||
|
||||
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();
|
||||
|
||||
|
||||
@@ -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: [] }),
|
||||
|
||||
@@ -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 }),
|
||||
};
|
||||
|
||||
@@ -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") && (
|
||||
<>
|
||||
<SettingsInput
|
||||
testId="server-name-input"
|
||||
name="name"
|
||||
type="text"
|
||||
label={t(I18nKey.SETTINGS$MCP_SERVER_NAME)}
|
||||
className="w-full min-w-0"
|
||||
showOptionalTag
|
||||
defaultValue={server?.name || ""}
|
||||
placeholder="my-search-server"
|
||||
/>
|
||||
|
||||
<SettingsInput
|
||||
testId="url-input"
|
||||
name="url"
|
||||
|
||||
@@ -41,6 +41,7 @@ export function useAddMcpServer() {
|
||||
|
||||
if (server.type === "sse") {
|
||||
const sseServer: MCPSSEServer = {
|
||||
...(server.name && { name: server.name }),
|
||||
url: server.url!,
|
||||
...(server.api_key && { api_key: server.api_key }),
|
||||
};
|
||||
@@ -55,6 +56,7 @@ export function useAddMcpServer() {
|
||||
newConfig.stdio_servers.push(stdioServer);
|
||||
} else if (server.type === "shttp") {
|
||||
const shttpServer: MCPSHTTPServer = {
|
||||
...(server.name && { name: server.name }),
|
||||
url: server.url!,
|
||||
...(server.api_key && { api_key: server.api_key }),
|
||||
...(server.timeout !== undefined && { timeout: server.timeout }),
|
||||
|
||||
@@ -49,6 +49,7 @@ export function useUpdateMcpServer() {
|
||||
|
||||
if (serverType === "sse") {
|
||||
const sseServer: MCPSSEServer = {
|
||||
...(server.name && { name: server.name }),
|
||||
url: server.url!,
|
||||
...(server.api_key && { api_key: server.api_key }),
|
||||
};
|
||||
@@ -63,6 +64,7 @@ export function useUpdateMcpServer() {
|
||||
newConfig.stdio_servers[index] = stdioServer;
|
||||
} else if (serverType === "shttp") {
|
||||
const shttpServer: MCPSHTTPServer = {
|
||||
...(server.name && { name: server.name }),
|
||||
url: server.url!,
|
||||
...(server.api_key && { api_key: server.api_key }),
|
||||
...(server.timeout !== undefined && { timeout: server.timeout }),
|
||||
|
||||
@@ -1733,6 +1733,23 @@
|
||||
"uk": "Назва",
|
||||
"ca": "Nom"
|
||||
},
|
||||
"SETTINGS$MCP_SERVER_NAME": {
|
||||
"en": "Server name",
|
||||
"ja": "サーバー名",
|
||||
"zh-CN": "服务器名称",
|
||||
"zh-TW": "伺服器名稱",
|
||||
"ko-KR": "서버 이름",
|
||||
"no": "Servernavn",
|
||||
"it": "Nome del server",
|
||||
"pt": "Nome do servidor",
|
||||
"es": "Nombre del servidor",
|
||||
"ar": "اسم الخادم",
|
||||
"fr": "Nom du serveur",
|
||||
"tr": "Sunucu adı",
|
||||
"de": "Servername",
|
||||
"uk": "Назва сервера",
|
||||
"ca": "Nom del servidor"
|
||||
},
|
||||
"SETTINGS$MCP_URL": {
|
||||
"en": "URL",
|
||||
"ja": "URL",
|
||||
|
||||
@@ -15,6 +15,7 @@ export type ProviderToken = {
|
||||
};
|
||||
|
||||
export type MCPSSEServer = {
|
||||
name?: string;
|
||||
url: string;
|
||||
api_key?: string;
|
||||
};
|
||||
@@ -27,6 +28,7 @@ export type MCPStdioServer = {
|
||||
};
|
||||
|
||||
export type MCPSHTTPServer = {
|
||||
name?: string;
|
||||
url: string;
|
||||
api_key?: string;
|
||||
timeout?: number;
|
||||
|
||||
+26
-2
@@ -34,6 +34,22 @@ function isDeprecatedLinearSse(
|
||||
type SdkMcpServerConfig = Record<string, SettingsValue>;
|
||||
type SdkMcpConfig = { mcpServers: Record<string, SdkMcpServerConfig> };
|
||||
|
||||
/**
|
||||
* 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) {
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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: "<redacted>",
|
||||
@@ -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}`,
|
||||
|
||||
Reference in New Issue
Block a user