mirror of
https://github.com/OpenHands/OpenHands.git
synced 2026-10-07 16:08:23 +08:00
Fix MCP server creation failing with 'No backend is configured.' on cloud backends (#1170)
Creating a Slack (or any other) MCP server on a cloud-active session
failed with the error 'No backend is configured.' The InstallServerModal's
pre-flight connectivity check called McpService.testServer, which went
through getAgentServerClientOptions → getEffectiveLocalBackend. On cloud
sessions there is no eligible local backend, so the helper threw
NoBackendAvailableError and the modal surfaced its message verbatim,
blocking the install entirely.
The MCP /api/mcp/test endpoint is local-agent-server only: it spawns
the stdio command (or opens an SSE/SHTTP connection) from that process's
environment. For cloud users, the MCP server would ultimately run inside
the cloud sandbox, which the browser can't reach pre-conversation-start,
so this pre-flight check has no useful cloud equivalent.
- McpService.testServer now short-circuits with a synthetic
{ ok: true, tools: [] } when the active backend is cloud. The save
flow (SettingsService.saveSettings → saveCloudSettings) is already
cloud-aware, so the install completes; any real MCP connection
failure surfaces inside the conversation runtime instead.
- CustomServerEditor hides the explicit 'Test connection' button (and
its testMessage) on cloud backends so users aren't shown a misleading
'0 tools found' success.
- Added a regression test asserting the cloud short-circuit returns
without calling the underlying MCPClient.testServer.
Co-authored-by: openhands <openhands@all-hands.dev>
This commit is contained in:
committed by
GitHub
co-authored by
openhands
parent
a1158f1e77
commit
449056f91f
@@ -1,5 +1,6 @@
|
||||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
import McpService from "#/api/mcp-service/mcp-service.api";
|
||||
import * as activeStore from "#/api/backend-registry/active-store";
|
||||
import type { MCPServerConfig } from "#/types/mcp-server";
|
||||
|
||||
// vi.mock factories are hoisted before imports, so spy functions must be
|
||||
@@ -27,6 +28,36 @@ vi.mock("#/api/agent-server-client-options", () => ({
|
||||
}),
|
||||
}));
|
||||
|
||||
vi.mock("#/api/backend-registry/active-store", () => ({
|
||||
getActiveBackend: vi.fn(),
|
||||
}));
|
||||
|
||||
const mockGetActiveBackend = vi.mocked(activeStore.getActiveBackend);
|
||||
|
||||
const localActive = () =>
|
||||
mockGetActiveBackend.mockReturnValue({
|
||||
backend: {
|
||||
id: "local-1",
|
||||
name: "Local",
|
||||
host: "http://localhost:3000",
|
||||
apiKey: "test-key",
|
||||
kind: "local",
|
||||
},
|
||||
orgId: null,
|
||||
});
|
||||
|
||||
const cloudActive = () =>
|
||||
mockGetActiveBackend.mockReturnValue({
|
||||
backend: {
|
||||
id: "cloud-1",
|
||||
name: "Cloud",
|
||||
host: "https://app.all-hands.dev",
|
||||
apiKey: "cloud-key",
|
||||
kind: "cloud",
|
||||
},
|
||||
orgId: null,
|
||||
});
|
||||
|
||||
const SERVER: MCPServerConfig = {
|
||||
id: "shttp-1",
|
||||
type: "shttp",
|
||||
@@ -36,6 +67,7 @@ const SERVER: MCPServerConfig = {
|
||||
describe("McpService.testServer", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
localActive();
|
||||
});
|
||||
|
||||
it("passes success responses through unchanged", async () => {
|
||||
@@ -51,7 +83,8 @@ describe("McpService.testServer", () => {
|
||||
// by the i18next no-escape prefix in the translation string, not here.
|
||||
mockTestServer.mockResolvedValue({
|
||||
ok: false,
|
||||
error: "Client error '401 Unauthorized' for url https://mcp.example.com/mcp",
|
||||
error:
|
||||
"Client error '401 Unauthorized' for url https://mcp.example.com/mcp",
|
||||
error_kind: "unknown",
|
||||
});
|
||||
|
||||
@@ -59,7 +92,8 @@ describe("McpService.testServer", () => {
|
||||
|
||||
expect(result).toEqual({
|
||||
ok: false,
|
||||
error: "Client error '401 Unauthorized' for url https://mcp.example.com/mcp",
|
||||
error:
|
||||
"Client error '401 Unauthorized' for url https://mcp.example.com/mcp",
|
||||
error_kind: "unknown",
|
||||
});
|
||||
});
|
||||
@@ -87,4 +121,18 @@ describe("McpService.testServer", () => {
|
||||
name: "my-server",
|
||||
});
|
||||
});
|
||||
|
||||
it("short-circuits with a synthetic ok response on cloud backends", async () => {
|
||||
// Regression: when the active backend is cloud, the local agent-server's
|
||||
// /api/mcp/test endpoint is not reachable. Previously, the helper threw
|
||||
// `NoBackendAvailableError("No backend is configured.")` which surfaced
|
||||
// in the install modal and blocked users from creating any MCP server
|
||||
// (e.g. Slack) on a cloud session.
|
||||
cloudActive();
|
||||
|
||||
const result = await McpService.testServer(SERVER);
|
||||
|
||||
expect(result).toEqual({ ok: true, tools: [] });
|
||||
expect(mockTestServer).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -4,6 +4,7 @@ import type {
|
||||
MCPTestResponse,
|
||||
} from "@openhands/typescript-client";
|
||||
import { getAgentServerClientOptions } from "../agent-server-client-options";
|
||||
import { getActiveBackend } from "../backend-registry/active-store";
|
||||
import type { MCPServerConfig } from "#/types/mcp-server";
|
||||
|
||||
function toMcpServerSpec(server: MCPServerConfig): MCPServerSpec {
|
||||
@@ -25,6 +26,19 @@ function toMcpServerSpec(server: MCPServerConfig): MCPServerSpec {
|
||||
|
||||
class McpService {
|
||||
static async testServer(server: MCPServerConfig): Promise<MCPTestResponse> {
|
||||
// The MCP connectivity-test endpoint lives on the local agent-server. It
|
||||
// spawns the configured stdio command / opens an SSE-or-SHTTP connection
|
||||
// from that process's environment. Cloud backends don't expose this
|
||||
// endpoint to the frontend — the MCP server would actually run inside the
|
||||
// cloud sandbox, which isn't reachable from the browser before the user
|
||||
// starts a conversation. Calling `getAgentServerClientOptions()` here for
|
||||
// a cloud-active session would throw `NoBackendAvailableError("No backend
|
||||
// is configured.")` and block the install flow entirely. Short-circuit
|
||||
// with a synthetic success so saving proceeds; any real connection
|
||||
// failure surfaces inside the conversation runtime instead.
|
||||
if (getActiveBackend().backend.kind === "cloud") {
|
||||
return { ok: true, tools: [] };
|
||||
}
|
||||
const { host, apiKey } = getAgentServerClientOptions();
|
||||
const client = new MCPClient({ host, ...(apiKey ? { apiKey } : {}) });
|
||||
try {
|
||||
|
||||
@@ -14,6 +14,7 @@ import { useAddMcpServer } from "#/hooks/mutation/use-add-mcp-server";
|
||||
import { useUpdateMcpServer } from "#/hooks/mutation/use-update-mcp-server";
|
||||
import { useDeleteMcpServer } from "#/hooks/mutation/use-delete-mcp-server";
|
||||
import { useTestMcpServer } from "#/hooks/mutation/use-test-mcp-server";
|
||||
import { useActiveBackend } from "#/contexts/active-backend-context";
|
||||
import { MCPServerConfig } from "#/types/mcp-server";
|
||||
import {
|
||||
displayErrorToast,
|
||||
@@ -53,6 +54,13 @@ export function CustomServerEditor({
|
||||
} = useTestMcpServer();
|
||||
const [showDeleteConfirm, setShowDeleteConfirm] = React.useState(false);
|
||||
|
||||
// The MCP connectivity-test endpoint only exists on the local agent-server.
|
||||
// For cloud backends `McpService.testServer` short-circuits with a synthetic
|
||||
// success so the save still completes; we hide the manual "Test connection"
|
||||
// button here so cloud users aren't shown a misleading "0 tools" result.
|
||||
const { backend } = useActiveBackend();
|
||||
const isCloudBackend = backend.kind === "cloud";
|
||||
|
||||
const isEditing = !!server.id;
|
||||
const isPending = isAdding || isUpdating || isDeleting;
|
||||
const isDismissBlocked = isPending || isTesting || showDeleteConfirm;
|
||||
@@ -163,9 +171,9 @@ export function CustomServerEditor({
|
||||
onCancel={onClose}
|
||||
onDelete={isEditing ? () => setShowDeleteConfirm(true) : undefined}
|
||||
isActionDisabled={isPending}
|
||||
onTest={handleTestClick}
|
||||
onTest={isCloudBackend ? undefined : handleTestClick}
|
||||
isTestPending={isTesting}
|
||||
testMessage={testMessage}
|
||||
testMessage={isCloudBackend ? null : testMessage}
|
||||
/>
|
||||
</div>
|
||||
</ModalBackdrop>
|
||||
|
||||
Reference in New Issue
Block a user