From 82ea2b609abc85ae5568a043242bb4b14a819ba0 Mon Sep 17 00:00:00 2001 From: OpenHands Bot Date: Fri, 12 Jun 2026 22:03:46 +0200 Subject: [PATCH] feat: save hosted MCP credentials as secrets (#1331) --- .github/workflows/docker.yml | 3 - AGENTS.md | 2 +- .../recommended-automations.test.tsx | 30 ++-- .../mcp-page/install-server-modal.test.tsx | 137 ++++++++++++++++-- __tests__/routes/mcp-page.test.tsx | 22 +-- __tests__/utils/mcp-marketplace-utils.test.ts | 112 ++++++-------- config/defaults.json | 3 +- docker/Dockerfile | 27 ---- package-lock.json | 8 +- package.json | 2 +- scripts/docker-build.mjs | 4 - .../mcp-page/install-server-modal.tsx | 94 +++++++++--- .../mutation/use-save-fields-as-secrets.ts | 18 ++- src/utils/mcp-marketplace-utils.ts | 42 ------ .../e2e/mock-llm/mock-llm-mcp-github.spec.ts | 104 +++++-------- 15 files changed, 317 insertions(+), 291 deletions(-) diff --git a/.github/workflows/docker.yml b/.github/workflows/docker.yml index 498dae5a54..404bb5abdb 100644 --- a/.github/workflows/docker.yml +++ b/.github/workflows/docker.yml @@ -90,12 +90,10 @@ jobs: AGENT_SERVER_VERSION=$(node -p "require('./config/defaults.json').versions.agentServer") AGENT_SERVER_IMAGE_BASE=$(node -p "require('./config/defaults.json').images.agentServer") AUTOMATION_VERSION=$(node -p "require('./config/defaults.json').versions.automation") - GITHUB_MCP_SERVER_VERSION=$(node -p "require('./config/defaults.json').versions.githubMcpServer") AGENT_CANVAS_VERSION=$(node -p "require('./package.json').version") echo "agent_server_version=$AGENT_SERVER_VERSION" >> "$GITHUB_OUTPUT" echo "default_agent_server_image=${AGENT_SERVER_IMAGE_BASE}:${AGENT_SERVER_VERSION}-python" >> "$GITHUB_OUTPUT" echo "default_automation_version=$AUTOMATION_VERSION" >> "$GITHUB_OUTPUT" - echo "github_mcp_server_version=$GITHUB_MCP_SERVER_VERSION" >> "$GITHUB_OUTPUT" echo "agent_canvas_version=$AGENT_CANVAS_VERSION" >> "$GITHUB_OUTPUT" - name: Compute metadata and tags @@ -199,7 +197,6 @@ jobs: build-args: | AGENT_SERVER_IMAGE=${{ steps.prep.outputs.agent_server_image }} AUTOMATION_VERSION=${{ steps.prep.outputs.automation_version }} - GITHUB_MCP_SERVER_VERSION=${{ steps.config.outputs.github_mcp_server_version }} AGENT_CANVAS_VERSION=${{ steps.config.outputs.agent_canvas_version }} OPENHANDS_BUILD_GIT_SHA=${{ env.RELEVANT_SHA }} OPENHANDS_BUILD_GIT_REF=${{ env.RELEVANT_REF }} diff --git a/AGENTS.md b/AGENTS.md index ada5ecd2f7..9af2ce3b80 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -573,7 +573,7 @@ When adding code that needs a new string, decide up front which rule it falls un - Custom secrets are NOT auto-attached by the agent-server. `POST /api/conversations` only persists what the client sends in `request.secrets`; the persisted secrets store (`/api/settings/secrets`) is never read at conversation-start. `buildStartConversationRequestWithEncryptedSettings` enumerates `SecretsService.getSecrets()` and turns each entry into a `LookupSecret` whose `url` points back at `/api/settings/secrets/{name}` and whose `headers` carry `X-Session-API-Key` for auth. Pre-1.21.x agent-server SDKs would silently drop that header during validation when `secrets_encrypted=true` (the cipher in the validation context tried to `cipher.decrypt(plaintext_session_key)`, failed, and the validator removed the header — the conversation runtime then got 401s for every saved secret). The SDK fix preserves plaintext header values when decryption fails; if you still see saved secrets unavailable inside a conversation, verify the running agent-server bundles a `LookupSecret._validate_secrets` that falls back to plaintext on decrypt failure. - MCP page layout: MCP is a **top-level** nav entry at `/mcp` (rendered by `src/routes/mcp.tsx`), shown right below "Skills" in `src/components/features/sidebar/sidebar.tsx`. The legacy `/settings/mcp` route still works as a redirect via `src/routes/mcp-settings-redirect.tsx`, and `src/routes/mcp-settings.tsx` re-exports the new page so the published `MCPSettings` library symbol (in `src/components/settings/index.ts`) keeps the same shape. Marketplace catalog data and MCP logo mappings live in the MCP-capable entries from `@openhands/extensions/integrations`; the Slack API catalog option should point at `https://github.com/zencoderai/slack-mcp-server` and use `@zencoderai/slack-mcp-server`. Deprecated marketplace entries removed upstream (for example GitLab / Google Maps / Postgres / Puppeteer / SQLite) should disappear from the marketplace grid. The Installed section still needs to render and search arbitrary non-catalog custom servers via the raw server `name` / `command` fallback in `src/utils/mcp-marketplace-utils.ts` + `InstalledServerCard`. Tavily is a regular stdio MCP entry (`tavily-mcp` + `TAVILY_API_KEY`), not a special built-in sentinel anymore. Components are colocated under `src/components/features/mcp-page/` and reuse the existing `MCPServerForm` for the "Add custom server" / edit flow. -- MCP catalog runtime patching: `getMcpMarketplaceCatalog()` in `src/utils/mcp-marketplace-utils.ts` pipes every catalog entry through patch functions before the UI sees it. Two patches exist: `patchLinearEntry` (rewrites the deprecated Linear SSE endpoint to streamable HTTP) and `patchGitHubEntry` (rewrites the `docker run` transport to the native `github-mcp-server stdio` binary, only when `getDeploymentMode() === "docker"`). The `getDeploymentMode()` helper is exported from `src/api/agent-server-adapter.ts` and reads the `mode` field from the runtime services info. The Docker image pre-installs the `github-mcp-server` Go binary at `/usr/local/bin/github-mcp-server` via a dedicated Dockerfile download stage (`github-mcp-download`). This avoids a Docker-in-Docker requirement — GitHub is the only catalog entry that uses `docker` as its stdio command; all others use `npx` or `uvx`. When adding similar patches for other entries, follow the same pattern: guard on `entry.id`, check environment conditionally, spread immutably. +- MCP catalog runtime patching: `getMcpMarketplaceCatalog()` in `src/utils/mcp-marketplace-utils.ts` still patches Linear from the deprecated SSE endpoint to streamable HTTP. Do not reintroduce the old GitHub Docker/native-binary patch: GitHub MCP is hosted (`https://api.githubcopilot.com/mcp/`) via `@openhands/extensions` 0.5.x, and the Docker image no longer bundles `/usr/local/bin/github-mcp-server`. - Library packaging notes: - Public npm entrypoints now come from `src/index.ts` → `src/lib/index.ts`, with domain barrels under `src/components/{conversation,terminal,browser,files,settings,sidebar}/index.ts`. diff --git a/__tests__/components/automations/recommended-automations.test.tsx b/__tests__/components/automations/recommended-automations.test.tsx index a95c570a71..e1154c174b 100644 --- a/__tests__/components/automations/recommended-automations.test.tsx +++ b/__tests__/components/automations/recommended-automations.test.tsx @@ -65,6 +65,8 @@ const localBackend: Backend = { kind: "local", }; +const GITHUB_HOSTED_MCP_URL = "https://api.githubcopilot.com/mcp/"; + const cloudBackend: Backend = { id: "cloud-backend", name: "Cloud", @@ -103,9 +105,8 @@ function settingsWithGithubMcp() { return settingsWithMcpConfig({ mcpServers: { github: { - command: "npx", - args: ["-y", "@modelcontextprotocol/server-github"], - env: { GITHUB_PERSONAL_ACCESS_TOKEN: "github-token" }, + url: GITHUB_HOSTED_MCP_URL, + auth: "github-token", }, }, }); @@ -369,16 +370,16 @@ describe("recommended automations", () => { const modal = await screen.findByTestId("mcp-install-modal"); expect(modal).toHaveAttribute("data-marketplace-id", "github"); - expect( - screen.getByTestId("mcp-install-field-command-readonly"), - ).toHaveValue( - "docker run -i --rm -e GITHUB_PERSONAL_ACCESS_TOKEN ghcr.io/github/github-mcp-server", + expect(screen.getByTestId("mcp-install-field-url")).toHaveValue( + GITHUB_HOSTED_MCP_URL, ); + expect(screen.getByTestId("mcp-install-field-api_key")).toBeInTheDocument(); expect( - screen.getByTestId("mcp-install-field-GITHUB_PERSONAL_ACCESS_TOKEN"), - ).toBeInTheDocument(); - expect(screen.queryByTestId("mcp-install-field-url")).toBeNull(); - expect(screen.queryByTestId("mcp-install-field-api_key")).toBeNull(); + screen.queryByTestId("mcp-install-field-command-readonly"), + ).toBeNull(); + expect( + screen.queryByTestId("mcp-install-field-GITHUB_PERSONAL_ACCESS_TOKEN"), + ).toBeNull(); expect(mockCreateConversationMutate).not.toHaveBeenCalled(); }); @@ -442,10 +443,9 @@ describe("recommended automations", () => { ); await screen.findByTestId("mcp-install-modal"); - fireEvent.change( - screen.getByTestId("mcp-install-field-GITHUB_PERSONAL_ACCESS_TOKEN"), - { target: { value: "github-token" } }, - ); + fireEvent.change(screen.getByTestId("mcp-install-field-api_key"), { + target: { value: "github-token" }, + }); fireEvent.click(screen.getByTestId("mcp-install-submit")); await waitFor(() => expect(saveSpy).toHaveBeenCalledTimes(1)); 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 8473b4429e..b7dcf2cac1 100644 --- a/__tests__/components/features/mcp-page/install-server-modal.test.tsx +++ b/__tests__/components/features/mcp-page/install-server-modal.test.tsx @@ -318,9 +318,7 @@ describe("InstallServerModal", () => { await screen.findByTestId("mcp-install-modal"); // Wait for settings to load so the mutation isn't a no-op. - await waitFor(() => - expect(SettingsService.getSettings).toHaveBeenCalled(), - ); + await waitFor(() => expect(SettingsService.getSettings).toHaveBeenCalled()); fireEvent.click(screen.getByTestId("mcp-install-submit")); @@ -371,9 +369,7 @@ describe("InstallServerModal", () => { renderWith(); await screen.findByTestId("mcp-install-modal"); - await waitFor(() => - expect(SettingsService.getSettings).toHaveBeenCalled(), - ); + await waitFor(() => expect(SettingsService.getSettings).toHaveBeenCalled()); fireEvent.click(screen.getByTestId("mcp-install-submit")); @@ -414,9 +410,7 @@ describe("InstallServerModal", () => { renderWith(); await screen.findByTestId("mcp-install-modal"); - await waitFor(() => - expect(SettingsService.getSettings).toHaveBeenCalled(), - ); + await waitFor(() => expect(SettingsService.getSettings).toHaveBeenCalled()); fireEvent.click(screen.getByTestId("mcp-install-submit")); @@ -482,6 +476,33 @@ describe("InstallServerModal", () => { ], } as unknown as MarketplaceEntry; + const SHTTP_ENTRY = { + id: "synthetic-shttp-secret", + kind: "mcp", + name: "Synthetic Hosted Server", + description: "Hosted server used to test credential secret saving.", + iconBg: "#000000", + defaultConnectionOptionId: "api", + connectionOptions: [ + { + id: "api", + provider: "mcp", + transport: { + kind: "shttp", + url: "https://example.com/mcp", + }, + auth: { + strategy: "api_key", + credentialLabel: "Personal access token", + credentialPlaceholder: "pat_...", + credentialHelp: "Token from the provider settings.", + credentialSecretName: "PROVIDER_PERSONAL_ACCESS_TOKEN", + saveCredentialAsSecretByDefault: true, + }, + }, + ], + } as unknown as MarketplaceEntry; + describe("InstallServerModal — save as secret", () => { beforeEach(() => { vi.spyOn(SecretsService, "createSecret").mockResolvedValue(); @@ -548,7 +569,9 @@ describe("InstallServerModal", () => { const onClose = vi.fn(); renderWith(); await screen.findByTestId("mcp-install-modal"); - await waitFor(() => expect(SettingsService.getSettings).toHaveBeenCalled()); + await waitFor(() => + expect(SettingsService.getSettings).toHaveBeenCalled(), + ); // Fill in the required password field (API_KEY is pre-checked as secret). fireEvent.change(screen.getByTestId("mcp-install-field-API_KEY"), { @@ -572,12 +595,97 @@ describe("InstallServerModal", () => { ); }); + it("saves hosted MCP credentials as named secrets when configured", async () => { + vi.spyOn(SettingsService, "saveSettings").mockResolvedValue(true); + const onClose = vi.fn(); + renderWith(); + await screen.findByTestId("mcp-install-modal"); + await waitFor(() => + expect(SettingsService.getSettings).toHaveBeenCalled(), + ); + + expect(screen.getByTestId("mcp-install-field-url")).toHaveValue( + "https://example.com/mcp", + ); + expect(screen.getByLabelText("Personal access token")).toHaveAttribute( + "placeholder", + "pat_...", + ); + expect( + screen.getByText("Token from the provider settings."), + ).toBeInTheDocument(); + + const toggle = screen.getByTestId( + "mcp-install-save-secret-PROVIDER_PERSONAL_ACCESS_TOKEN", + ); + expect(toggle.querySelector("input[type='checkbox']")).toBeChecked(); + + fireEvent.change(screen.getByTestId("mcp-install-field-api_key"), { + target: { value: "hosted-token" }, + }); + fireEvent.click(screen.getByTestId("mcp-install-submit")); + + await waitFor(() => expect(onClose).toHaveBeenCalledTimes(1)); + await waitFor(() => + expect(SecretsService.createSecret).toHaveBeenCalledWith( + "PROVIDER_PERSONAL_ACCESS_TOKEN", + "hosted-token", + "Personal access token", + ), + ); + }); + + it("waits for hosted credential secrets before reporting install success", async () => { + vi.spyOn(SettingsService, "saveSettings").mockResolvedValue(true); + let resolveSecret!: () => void; + const secretSaved = new Promise((resolve) => { + resolveSecret = resolve; + }); + vi.spyOn(SecretsService, "createSecret").mockReturnValue(secretSaved); + const onClose = vi.fn(); + const onSuccess = vi.fn(); + + renderWith( + , + ); + await screen.findByTestId("mcp-install-modal"); + await waitFor(() => + expect(SettingsService.getSettings).toHaveBeenCalled(), + ); + + fireEvent.change(screen.getByTestId("mcp-install-field-api_key"), { + target: { value: "hosted-token" }, + }); + fireEvent.click(screen.getByTestId("mcp-install-submit")); + + await waitFor(() => + expect(SecretsService.createSecret).toHaveBeenCalledWith( + "PROVIDER_PERSONAL_ACCESS_TOKEN", + "hosted-token", + "Personal access token", + ), + ); + expect(onSuccess).not.toHaveBeenCalled(); + expect(onClose).not.toHaveBeenCalled(); + + resolveSecret(); + + await waitFor(() => expect(onSuccess).toHaveBeenCalledTimes(1)); + expect(onClose).toHaveBeenCalledTimes(1); + }); + it("does not call createSecret when all toggles are unchecked before install", async () => { vi.spyOn(SettingsService, "saveSettings").mockResolvedValue(true); const onClose = vi.fn(); renderWith(); await screen.findByTestId("mcp-install-modal"); - await waitFor(() => expect(SettingsService.getSettings).toHaveBeenCalled()); + await waitFor(() => + expect(SettingsService.getSettings).toHaveBeenCalled(), + ); fireEvent.change(screen.getByTestId("mcp-install-field-API_KEY"), { target: { value: "my-api-key" }, @@ -596,7 +704,7 @@ describe("InstallServerModal", () => { expect(SecretsService.createSecret).not.toHaveBeenCalled(); }); - it("closes the modal even when the background secret save fails", async () => { + it("closes the modal even when the secret save fails", async () => { vi.spyOn(SettingsService, "saveSettings").mockResolvedValue(true); vi.spyOn(SecretsService, "createSecret").mockRejectedValue( new Error("forbidden"), @@ -604,7 +712,9 @@ describe("InstallServerModal", () => { const onClose = vi.fn(); renderWith(); await screen.findByTestId("mcp-install-modal"); - await waitFor(() => expect(SettingsService.getSettings).toHaveBeenCalled()); + await waitFor(() => + expect(SettingsService.getSettings).toHaveBeenCalled(), + ); fireEvent.change(screen.getByTestId("mcp-install-field-API_KEY"), { target: { value: "my-api-key" }, @@ -619,5 +729,4 @@ describe("InstallServerModal", () => { ).not.toBeInTheDocument(); }); }); - }); diff --git a/__tests__/routes/mcp-page.test.tsx b/__tests__/routes/mcp-page.test.tsx index c144c7a60c..6ff20324a5 100644 --- a/__tests__/routes/mcp-page.test.tsx +++ b/__tests__/routes/mcp-page.test.tsx @@ -280,7 +280,7 @@ describe("MCPPage", () => { expect(sent.mcp_config).toBeNull(); }); - it("shows the catalog description and command line on installed server cards", async () => { + it("shows the catalog description and URL on installed server cards", async () => { vi.spyOn(SettingsService, "getSettings").mockResolvedValue( buildSettings({ agent_settings: { @@ -288,16 +288,8 @@ describe("MCPPage", () => { mcp_config: { mcpServers: { github: { - command: "docker", - args: [ - "run", - "-i", - "--rm", - "-e", - "GITHUB_PERSONAL_ACCESS_TOKEN", - "ghcr.io/github/github-mcp-server", - ], - env: { GITHUB_PERSONAL_ACCESS_TOKEN: "github_pat_test" }, + url: "https://api.githubcopilot.com/mcp/", + auth: "github_pat_test", }, }, }, @@ -309,15 +301,13 @@ describe("MCPPage", () => { const card = await screen.findByTestId("mcp-server-item"); expect( - within(card).getByTestId("mcp-server-description-stdio-0"), + within(card).getByTestId("mcp-server-description-shttp-0"), ).toHaveTextContent( "Search code, manage issues and pull requests, and inspect repos via the GitHub API.", ); expect( - within(card).getByTestId("mcp-server-detail-stdio-0"), - ).toHaveTextContent( - "docker run -i --rm -e GITHUB_PERSONAL_ACCESS_TOKEN ghcr.io/github/github-mcp-server", - ); + within(card).getByTestId("mcp-server-detail-shttp-0"), + ).toHaveTextContent("https://api.githubcopilot.com/mcp/"); }); it("shows Tavily marketplace toggle as add-only when installed", async () => { diff --git a/__tests__/utils/mcp-marketplace-utils.test.ts b/__tests__/utils/mcp-marketplace-utils.test.ts index 498cc3a291..bebeb8082d 100644 --- a/__tests__/utils/mcp-marketplace-utils.test.ts +++ b/__tests__/utils/mcp-marketplace-utils.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, it, vi } from "vitest"; +import { describe, expect, it } from "vitest"; import { findCatalogEntryForServer, findInstalledMatch, @@ -9,15 +9,8 @@ import { isMarketplaceEntryAvailable, marketplaceEntryMatchesQuery, } from "#/utils/mcp-marketplace-utils"; -import * as agentServerAdapter from "#/api/agent-server-adapter"; import { INTEGRATION_CATALOG as MCP_MARKETPLACE } from "@openhands/extensions/integrations"; -vi.mock("#/api/agent-server-adapter", async (importOriginal) => { - const actual = - await importOriginal(); - return { ...actual, getDeploymentMode: vi.fn(() => null) }; -}); - const mcpMarketplace = getMcpMarketplaceCatalog(MCP_MARKETPLACE); const slackEntry = mcpMarketplace.find((e) => e.id === "slack")!; const tavilyEntry = mcpMarketplace.find((e) => e.id === "tavily")!; @@ -106,32 +99,36 @@ describe("getInstallableMcpConnectionOption", () => { }); it("returns undefined for an OAuth-only entry (no locally installable option)", () => { - const oauthOnlyEntry: Parameters[0] = - { - ...slackEntry, - id: "oauth-only", - defaultConnectionOptionId: "oauth", - connectionOptions: [ - { - id: "oauth", - provider: "mcp", - auth: { strategy: "oauth2" }, - transport: { kind: "shttp", url: "https://example.com/mcp" }, - } as Parameters[0]["connectionOptions"][number], - ], - }; + const oauthOnlyEntry: Parameters< + typeof getInstallableMcpConnectionOption + >[0] = { + ...slackEntry, + id: "oauth-only", + defaultConnectionOptionId: "oauth", + connectionOptions: [ + { + id: "oauth", + provider: "mcp", + auth: { strategy: "oauth2" }, + transport: { kind: "shttp", url: "https://example.com/mcp" }, + } as Parameters< + typeof getInstallableMcpConnectionOption + >[0]["connectionOptions"][number], + ], + }; const option = getInstallableMcpConnectionOption(oauthOnlyEntry); expect(option).toBeUndefined(); }); it("returns undefined when the entry has no MCP connection options", () => { - const noOptionsEntry: Parameters[0] = - { - ...slackEntry, - id: "no-mcp", - defaultConnectionOptionId: undefined, - connectionOptions: [], - }; + const noOptionsEntry: Parameters< + typeof getInstallableMcpConnectionOption + >[0] = { + ...slackEntry, + id: "no-mcp", + defaultConnectionOptionId: undefined, + connectionOptions: [], + }; const option = getInstallableMcpConnectionOption(noOptionsEntry); expect(option).toBeUndefined(); }); @@ -262,50 +259,37 @@ describe("findCatalogEntryForServer", () => { }); }); -describe("patchGitHubEntry (via getMcpMarketplaceCatalog)", () => { - const mockedGetDeploymentMode = vi.mocked( - agentServerAdapter.getDeploymentMode, - ); - - function getGitHubStdioTransport(catalog: ReturnType) { +describe("GitHub hosted MCP entry", () => { + function getGitHubTransport( + catalog: ReturnType, + ) { const github = catalog.find((e) => e.id === "github"); expect(github).toBeDefined(); - const option = github!.connectionOptions.find( - (o) => o.transport?.kind === "stdio", - ); - expect(option?.transport?.kind).toBe("stdio"); - const transport = option!.transport!; - if (transport.kind !== "stdio") throw new Error("expected stdio"); + const transport = getDefaultMcpTransport(github!); + expect(transport?.kind).toBe("shttp"); + if (transport?.kind !== "shttp") throw new Error("expected shttp"); return transport; } - it("leaves the GitHub entry unchanged when not in docker mode", () => { - mockedGetDeploymentMode.mockReturnValue(null); - const transport = getGitHubStdioTransport( + it("uses GitHub's hosted streamable HTTP endpoint", () => { + const transport = getGitHubTransport( getMcpMarketplaceCatalog(MCP_MARKETPLACE), ); - expect(transport.command).toBe("docker"); - expect(transport.args[0]).toBe("run"); + expect(transport.url).toBe("https://api.githubcopilot.com/mcp/"); }); - it("rewrites the GitHub command to native binary in docker mode", () => { - mockedGetDeploymentMode.mockReturnValue("docker"); - const transport = getGitHubStdioTransport( - getMcpMarketplaceCatalog(MCP_MARKETPLACE), + it("matches installed hosted GitHub servers by URL", () => { + const github = getMcpMarketplaceCatalog(MCP_MARKETPLACE).find( + (e) => e.id === "github", + )!; + const match = findCatalogEntryForServer( + { + id: "shttp-0", + type: "shttp", + url: "https://api.githubcopilot.com/mcp/", + }, + [github], ); - expect(transport.command).toBe("github-mcp-server"); - expect(transport.args).toEqual(["stdio"]); - }); - - it("does not affect other entries in docker mode", () => { - mockedGetDeploymentMode.mockReturnValue("docker"); - const catalog = getMcpMarketplaceCatalog(MCP_MARKETPLACE); - const tavily = catalog.find((e) => e.id === "tavily"); - const stdioOption = tavily?.connectionOptions.find( - (o) => o.transport?.kind === "stdio", - ); - expect(stdioOption?.transport?.kind).toBe("stdio"); - if (stdioOption?.transport?.kind !== "stdio") throw new Error("expected stdio"); - expect(stdioOption.transport.command).not.toBe("github-mcp-server"); + expect(match?.id).toBe("github"); }); }); diff --git a/config/defaults.json b/config/defaults.json index e6fdfa971b..0ffe79e7f6 100644 --- a/config/defaults.json +++ b/config/defaults.json @@ -5,8 +5,7 @@ "agentServer": "1.28.1", "agentCanvas": "1.0.0-rc.6", "automation": "1.0.0a9", - "automationSdk": "1.28.1", - "githubMcpServer": "1.2.0" + "automationSdk": "1.28.1" }, "images": { diff --git a/docker/Dockerfile b/docker/Dockerfile index 348f845209..977cb8b0c8 100644 --- a/docker/Dockerfile +++ b/docker/Dockerfile @@ -66,28 +66,6 @@ RUN node -e " \ require('fs').writeFileSync('/tmp/defaults.env', lines.join('\n') + '\n'); \ " -# ── Stage 1c: Download github-mcp-server binary ───────────────────────────── -# The upstream GitHub MCP server is a Go binary distributed via GitHub Releases. -# The @openhands/extensions catalog entry uses `docker run` as the default -# transport, which fails inside the agent-canvas Docker container (no Docker -# daemon). Pre-installing the binary lets the frontend patch the catalog entry -# to use the native binary instead. -FROM node:24-slim AS github-mcp-download - -ARG TARGETARCH -ARG GITHUB_MCP_SERVER_VERSION - -RUN apt-get update && apt-get install -y --no-install-recommends curl ca-certificates && rm -rf /var/lib/apt/lists/* - -# Map Docker TARGETARCH (amd64/arm64) to GitHub release naming (x86_64/arm64). -# The *) fallback defaults to x86_64 for any unrecognised architecture so the -# build doesn't silently produce an image without the binary. If a new arch is -# added to the Docker build matrix, add an explicit case here. -RUN ARCH_SUFFIX=$(case "${TARGETARCH}" in amd64) echo "x86_64";; arm64) echo "arm64";; *) echo "x86_64";; esac) && \ - curl -fsSL "https://github.com/github/github-mcp-server/releases/download/v${GITHUB_MCP_SERVER_VERSION}/github-mcp-server_Linux_${ARCH_SUFFIX}.tar.gz" \ - | tar -xz -C /usr/local/bin github-mcp-server && \ - chmod +x /usr/local/bin/github-mcp-server - # ── Stage 2: Combined image ────────────────────────────────────────────────── FROM ${AGENT_SERVER_IMAGE} AS final @@ -122,11 +100,6 @@ RUN if command -v apt-get >/dev/null 2>&1; then \ RUN uv pip install --system "openhands-automation==${AUTOMATION_VERSION}" 2>/dev/null \ || pip install --no-cache-dir "openhands-automation==${AUTOMATION_VERSION}" -# Install pre-built github-mcp-server binary so the GitHub MCP integration -# works without Docker-in-Docker (the catalog entry is patched at runtime to -# use this binary instead of `docker run`). -COPY --from=github-mcp-download /usr/local/bin/github-mcp-server /usr/local/bin/github-mcp-server - # Copy the frontend build output. # react-router.config.ts unpacks build/client/ into build/ for non-Vercel builds. COPY --from=frontend-build /build/build /opt/agent-canvas/frontend diff --git a/package-lock.json b/package-lock.json index 37a818eeb3..297f8e606e 100644 --- a/package-lock.json +++ b/package-lock.json @@ -12,7 +12,7 @@ "@heroui/react": "2.8.10", "@microlink/react-json-view": "1.31.20", "@monaco-editor/react": "4.7.0", - "@openhands/extensions": "0.4.2", + "@openhands/extensions": "0.5.0", "@openhands/typescript-client": "1.24.3", "@react-router/node": "7.17.0", "@react-router/serve": "7.17.0", @@ -3468,9 +3468,9 @@ "license": "MIT" }, "node_modules/@openhands/extensions": { - "version": "0.4.2", - "resolved": "https://registry.npmjs.org/@openhands/extensions/-/extensions-0.4.2.tgz", - "integrity": "sha512-sZ6EcW3lE+28HzXJx4VjYs4snx8cdMMk3nJzcf+fy7flwMXpV+8EIO1p62TmUjZtHpCkpGfM6DuGzEFMVB6nJQ==", + "version": "0.5.0", + "resolved": "https://registry.npmjs.org/@openhands/extensions/-/extensions-0.5.0.tgz", + "integrity": "sha512-k5JR2JJiipg9yCqEwDU67x458PKdSBp9tt0xbzncqtjgC+eKlsQQZ8eSayXPqJPCo9cv7dRZfxX7WW259fLu7g==", "license": "MIT", "engines": { "node": ">=18.20.0" diff --git a/package.json b/package.json index 937d1a73d6..89cc53896e 100644 --- a/package.json +++ b/package.json @@ -23,7 +23,7 @@ "@heroui/react": "2.8.10", "@microlink/react-json-view": "1.31.20", "@monaco-editor/react": "4.7.0", - "@openhands/extensions": "0.4.2", + "@openhands/extensions": "0.5.0", "@openhands/typescript-client": "1.24.3", "@react-router/node": "7.17.0", "@react-router/serve": "7.17.0", diff --git a/scripts/docker-build.mjs b/scripts/docker-build.mjs index e220d5ae8a..93338a988e 100644 --- a/scripts/docker-build.mjs +++ b/scripts/docker-build.mjs @@ -25,7 +25,6 @@ const config = JSON.parse( const agentServerImage = `${config.images.agentServer}:${config.versions.agentServer}-python`; const automationVersion = config.versions.automation; -const githubMcpServerVersion = config.versions.githubMcpServer; // Parse CLI: --tag and everything after -- is passed to docker build let tag = "agent-canvas:local"; @@ -51,8 +50,6 @@ const cmd = [ `AGENT_SERVER_IMAGE=${agentServerImage}`, "--build-arg", `AUTOMATION_VERSION=${automationVersion}`, - "--build-arg", - `GITHUB_MCP_SERVER_VERSION=${githubMcpServerVersion}`, "-t", tag, ...extraArgs, @@ -61,7 +58,6 @@ const cmd = [ console.log(`Agent Server image : ${agentServerImage}`); console.log(`Automation version : ${automationVersion}`); -console.log(`GitHub MCP Server : ${githubMcpServerVersion}`); console.log(`Tag : ${tag}`); console.log(`\n$ ${cmd.join(" ")}\n`); diff --git a/src/components/features/mcp-page/install-server-modal.tsx b/src/components/features/mcp-page/install-server-modal.tsx index 523d9deae8..5253bec6ce 100644 --- a/src/components/features/mcp-page/install-server-modal.tsx +++ b/src/components/features/mcp-page/install-server-modal.tsx @@ -9,7 +9,10 @@ import { BrandButton } from "#/components/features/settings/brand-button"; import { SettingsInput } from "#/components/features/settings/settings-input"; import { SaveAsSecretToggle } from "#/components/features/mcp-page/save-as-secret-toggle"; import { I18nKey } from "#/i18n/declaration"; -import type { IntegrationCatalogEntry as MarketplaceEntry } from "@openhands/extensions/integrations"; +import type { + IntegrationCatalogEntry as MarketplaceEntry, + MarketplaceField, +} from "@openhands/extensions/integrations"; import { McpLogoBadge } from "#/components/features/mcp-logo-badge"; import { MCPServerConfig } from "#/types/mcp-server"; import { useAddMcpServer } from "#/hooks/mutation/use-add-mcp-server"; @@ -100,6 +103,10 @@ function makeInitialState(entry: MarketplaceEntry): FieldState { } } else if (optionNeedsCredentialField(option)) { values.api_key = ""; + if (option?.auth.credentialSecretName) { + savedAsSecret.api_key = + option.auth.saveCredentialAsSecretByDefault ?? false; + } } return { values, errors: {}, savedAsSecret }; } @@ -129,10 +136,11 @@ export function InstallServerModal({ stateRef.current = state; const [globalError, setGlobalError] = React.useState(null); + const [isFinalizingInstall, setIsFinalizingInstall] = React.useState(false); const option = getInstallableMcpConnectionOption(entry); const template = option?.transport; - const isPending = isTesting || isAdding; + const isPending = isTesting || isAdding || isFinalizingInstall; const setValue = (key: string, value: string) => { setState((prev) => ({ @@ -150,6 +158,39 @@ export function InstallServerModal({ })); }; + const saveHostedCredentialAsSecret = (): Promise => { + const secretName = option?.auth.credentialSecretName; + const apiKey = stateRef.current.values.api_key?.trim(); + if (!secretName || !apiKey || !stateRef.current.savedAsSecret.api_key) { + return Promise.resolve(); + } + + const field: MarketplaceField = { + key: secretName, + label: option.auth.credentialLabel ?? secretName, + type: "password", + }; + return saveFieldsAsSecrets( + [field], + { [secretName]: apiKey }, + { [secretName]: true }, + ); + }; + + const saveSelectedSecrets = (): Promise => { + if (template?.kind === "stdio") { + return saveFieldsAsSecrets( + template.envFields ?? [], + stateRef.current.values, + stateRef.current.savedAsSecret, + ); + } + if (template?.kind === "shttp" || template?.kind === "sse") { + return saveHostedCredentialAsSecret(); + } + return Promise.resolve(); + }; + const makeTestErrorMessage = (failure: MCPTestFailure): string => { switch (failure.error_kind) { case "timeout": @@ -172,20 +213,15 @@ export function InstallServerModal({ addMcpServer(payload, { onSuccess: () => { displaySuccessToast(t(I18nKey.MCP$INSTALL_SUCCESS)); - onSuccess?.(entry); - onClose(); - - // Save checked envFields as secrets in the background so the - // Automation Server can access them without a separate manual step. - // Runs after onClose so failures don't block the modal from closing. - // Uses stateRef.current to avoid reading a stale closure snapshot. - if (template?.kind === "stdio") { - saveFieldsAsSecrets( - template.envFields ?? [], - stateRef.current.values, - stateRef.current.savedAsSecret, - ); - } + setIsFinalizingInstall(true); + void (async () => { + try { + await saveSelectedSecrets(); + } finally { + onSuccess?.(entry); + onClose(); + } + })(); }, onError: (err: unknown) => { const message = retrieveAxiosErrorMessage(err as AxiosError); @@ -290,6 +326,7 @@ export function InstallServerModal({ if (template?.kind === "shttp" || template?.kind === "sse") { const shouldRenderCredential = optionNeedsCredentialField(option); const apiKeyOptional = option ? isCredentialOptional(option) : false; + const credentialSecretName = option?.auth.credentialSecretName; return ( <> setValue("api_key", v)} - placeholder={t(I18nKey.SETTINGS$MCP_API_KEY_PLACEHOLDER)} + placeholder={ + option?.auth.credentialPlaceholder ?? + t(I18nKey.SETTINGS$MCP_API_KEY_PLACEHOLDER) + } showOptionalTag={apiKeyOptional} required={!apiKeyOptional} className="w-full" /> + {option?.auth.credentialHelp && ( +

+ {renderHelperText(option.auth.credentialHelp)} +

+ )} {state.errors.api_key && (

{state.errors.api_key}

)} + {credentialSecretName && ( + toggleSecret("api_key", v)} + /> + )} ) : null} @@ -453,6 +508,7 @@ export function InstallServerModal({ variant="secondary" onClick={onClose} testId="mcp-install-cancel" + isDisabled={isPending} > {t(I18nKey.BUTTON$CANCEL)} @@ -464,7 +520,7 @@ export function InstallServerModal({ > {isTesting ? t(I18nKey.MCP$VERIFYING) - : isAdding + : isAdding || isFinalizingInstall ? t(I18nKey.SETTINGS$SAVING) : t(I18nKey.MCP$INSTALL_BUTTON)} diff --git a/src/hooks/mutation/use-save-fields-as-secrets.ts b/src/hooks/mutation/use-save-fields-as-secrets.ts index 6afb9f2275..976ef8bb65 100644 --- a/src/hooks/mutation/use-save-fields-as-secrets.ts +++ b/src/hooks/mutation/use-save-fields-as-secrets.ts @@ -17,11 +17,13 @@ function formatKeyList(keys: string[]): string { } /** - * Returns a stable, fire-and-forget function that upserts checked envFields - * into the Secrets store. MCP server config and the Secrets store are - * separate — this bridges the gap so Automation Server can access credentials - * without a separate manual step. Internally, `SecretsService.createSecret` - * is an upsert, so existing secrets with the same name are overwritten safely. + * Returns a stable function that upserts checked envFields into the Secrets + * store. Callers may ignore the returned promise for background saves or await + * it when later work depends on the secret being present. MCP server config + * and the Secrets store are separate — this bridges the gap so Automation + * Server can access credentials without a separate manual step. Internally, + * `SecretsService.createSecret` is an upsert, so existing secrets with the + * same name are overwritten safely. */ export function useSaveFieldsAsSecrets() { const { t } = useTranslation("openhands"); @@ -32,13 +34,13 @@ export function useSaveFieldsAsSecrets() { envFields: MarketplaceField[], values: Record, savedAsSecret: Record, - ): void => { + ): Promise => { const fieldsToSave = envFields.filter( (field) => savedAsSecret[field.key] && (values[field.key] ?? "").trim(), ); - if (fieldsToSave.length === 0) return; + if (fieldsToSave.length === 0) return Promise.resolve(); - Promise.allSettled( + return Promise.allSettled( fieldsToSave.map((field) => SecretsService.createSecret( field.key, diff --git a/src/utils/mcp-marketplace-utils.ts b/src/utils/mcp-marketplace-utils.ts index 6533d0348f..9b7f7d7251 100644 --- a/src/utils/mcp-marketplace-utils.ts +++ b/src/utils/mcp-marketplace-utils.ts @@ -4,7 +4,6 @@ import type { IntegrationConnectionOption, IntegrationTransport, } from "@openhands/extensions/integrations"; -import { getDeploymentMode } from "#/api/agent-server-adapter"; export type { MarketplaceEntry }; @@ -104,52 +103,11 @@ function patchLinearEntry(entry: MarketplaceEntry): MarketplaceEntry { }; } -/** - * The upstream catalog ships the GitHub MCP server with `command: "docker"` - * (`docker run … ghcr.io/github/github-mcp-server`). This requires Docker- - * in-Docker when agent-canvas itself runs inside the Docker image, which - * isn't available. The Docker image pre-installs the native Go binary at - * `/usr/local/bin/github-mcp-server`, so we rewrite the transport to use - * it directly. - * - * Only applied when `getDeploymentMode()` returns `"docker"`. - */ -function patchGitHubEntry(entry: MarketplaceEntry): MarketplaceEntry { - if (entry.id !== "github") return entry; - if (getDeploymentMode() !== "docker") return entry; - return { - ...entry, - installHint: - "Requires a GitHub Personal Access Token (classic or fine-grained).", - // The upstream @openhands/extensions catalog defines the GitHub entry - // with `command: "docker"` (i.e. `docker run …`). We match on that - // exact value to replace it with the pre-installed native binary. - // If the upstream ever changes the command string, this patch becomes - // a no-op and the original transport is preserved — the worst case is - // the user falls back to the Docker-based transport (which still works - // outside the Docker image). - connectionOptions: entry.connectionOptions.map((option) => - option.transport?.kind === "stdio" && - option.transport.command === "docker" - ? { - ...option, - transport: { - ...option.transport, - command: "github-mcp-server", - args: ["stdio"], - }, - } - : option, - ), - }; -} - export function getMcpMarketplaceCatalog( catalog: MarketplaceEntry[], ): MarketplaceEntry[] { return catalog .map(patchLinearEntry) - .map(patchGitHubEntry) .filter((entry) => !!getDefaultMcpConnectionOption(entry)); } diff --git a/tests/e2e/mock-llm/mock-llm-mcp-github.spec.ts b/tests/e2e/mock-llm/mock-llm-mcp-github.spec.ts index 48a57272f2..d67a69b17d 100644 --- a/tests/e2e/mock-llm/mock-llm-mcp-github.spec.ts +++ b/tests/e2e/mock-llm/mock-llm-mcp-github.spec.ts @@ -4,17 +4,12 @@ * This test exercises the full MCP install flow — navigating to the MCP page, * finding the GitHub marketplace card, opening the install modal, filling in * the PAT field, and submitting. The `POST /api/mcp/test` endpoint is - * intercepted to return a mock success response so the test doesn't need a - * real `github-mcp-server` binary or Docker daemon. - * - * Runs in both npm (`mock-llm.config.ts`) and Docker (`mock-llm-docker.config.ts`) - * paths. In Docker mode, asserts that `patchGitHubEntry` rewrote the command to - * the pre-installed native binary (`github-mcp-server stdio`); in npm mode, - * asserts the original `docker run …` transport is shown. + * intercepted to return a mock success response so the test doesn't need to + * contact GitHub's hosted MCP endpoint. * * Verifies: * 1. The MCP page renders with the GitHub marketplace card visible - * 2. Clicking the card opens the install modal with the correct command + * 2. Clicking the card opens the install modal with the hosted endpoint * 3. Filling in the PAT and submitting succeeds (with mocked test endpoint) * 4. After install the GitHub server appears in the installed list * 5. The installed server can be deleted via the UI @@ -32,19 +27,7 @@ import { } from "./utils/mock-llm-helpers"; const FAKE_PAT = "github_pat_test_1234567890abcdef"; - -/** - * When running inside the Docker image (`--mode docker` in runtime services - * info), `patchGitHubEntry` rewrites the catalog command from `docker run …` - * to the pre-installed native binary. The Docker Playwright config sets - * `MOCK_LLM_DOCKER_IMAGE`, so we can use its presence to know which command - * value to expect. - */ -const IS_DOCKER_E2E = !!process.env.MOCK_LLM_DOCKER_IMAGE; -/** Pattern to match inside the read-only command field value. */ -const EXPECTED_COMMAND_PATTERN = IS_DOCKER_E2E - ? /github-mcp-server\s+stdio/ - : /docker/; +const GITHUB_HOSTED_MCP_URL = "https://api.githubcopilot.com/mcp/"; test.describe.configure({ mode: "serial" }); @@ -104,20 +87,15 @@ test.describe("MCP GitHub server install flow", () => { // Verify the modal is for the GitHub entry await expect(modal).toHaveAttribute("data-marketplace-id", "github"); - // The modal should show the command field (read-only). - // In Docker mode the field shows the patched native binary command; - // in the npm path it shows the original `docker run …` transport. - const commandField = page.getByTestId( - "mcp-install-field-command-readonly", - ); - await expect(commandField).toBeVisible(); - await expect(commandField).toHaveValue(EXPECTED_COMMAND_PATTERN); + // The modal should show the hosted streamable HTTP endpoint. + const urlField = page.getByTestId("mcp-install-field-url"); + await expect(urlField).toBeVisible(); + await expect(urlField).toHaveValue(GITHUB_HOSTED_MCP_URL); - // The PAT field should be present and empty - const patField = page.getByTestId( - "mcp-install-field-GITHUB_PERSONAL_ACCESS_TOKEN", - ); + // The PAT field should be present and empty. + const patField = page.getByTestId("mcp-install-field-api_key"); await expect(patField).toBeVisible(); + await expect(patField).toHaveValue(""); }); test("step 3: full install flow — fill PAT, submit, verify installed", async ({ @@ -128,8 +106,8 @@ test.describe("MCP GitHub server install flow", () => { await routeSessionApiKey(page); - // Intercept the MCP test endpoint to return success — we don't have - // the real github-mcp-server binary in the test environment. + // Intercept the MCP test endpoint to return success; the test environment + // should not contact GitHub's hosted MCP endpoint. await page.route("**/api/mcp/test", async (route) => { await route.fulfill({ status: 200, @@ -148,9 +126,7 @@ test.describe("MCP GitHub server install flow", () => { await expect(modal).toBeVisible({ timeout: 5_000 }); // Fill in the PAT — SettingsInput puts data-testid on the directly - const patInput = page.getByTestId( - "mcp-install-field-GITHUB_PERSONAL_ACCESS_TOKEN", - ); + const patInput = page.getByTestId("mcp-install-field-api_key"); await patInput.fill(FAKE_PAT); // Click install @@ -168,30 +144,22 @@ test.describe("MCP GitHub server install flow", () => { await expect(serverItems.first()).toBeVisible(); // Verify via the settings API that the server was actually persisted - const settingsResp = await page.request.get( - `${BACKEND_URL}/api/settings`, - { - headers: { "X-Session-API-Key": SESSION_API_KEY }, - }, - ); + const settingsResp = await page.request.get(`${BACKEND_URL}/api/settings`, { + headers: { "X-Session-API-Key": SESSION_API_KEY }, + }); expect(settingsResp.ok()).toBe(true); const settings = await settingsResp.json(); const mcpConfig = settings?.agent_settings?.mcp_config; expect(mcpConfig).toBeTruthy(); - // The GitHub server should be stored as a stdio server named "github" - // with the PAT in its env - const mcpServers = mcpConfig?.mcpServers ?? mcpConfig?.stdio_servers; + // The GitHub server should be stored as a hosted streamable HTTP server + // with the PAT saved as its auth credential. + const mcpServers = mcpConfig?.mcpServers ?? mcpConfig?.shttp_servers; expect(mcpServers).toBeTruthy(); - - // Check that there's a server named "github" somewhere in the config - const hasGithub = - mcpServers?.github != null || - (Array.isArray(mcpServers) && - mcpServers.some( - (s: Record) => s.name === "github", - )); - expect(hasGithub).toBe(true); + expect(mcpServers?.shttp).toMatchObject({ + url: GITHUB_HOSTED_MCP_URL, + auth: FAKE_PAT, + }); }); test("step 4: installed GitHub server can be deleted", async ({ page }) => { @@ -207,12 +175,9 @@ test.describe("MCP GitHub server install flow", () => { agent_settings_diff: { mcp_config: { mcpServers: { - github: { - command: "github-mcp-server", - args: ["stdio"], - env: { - GITHUB_PERSONAL_ACCESS_TOKEN: FAKE_PAT, - }, + shttp: { + url: GITHUB_HOSTED_MCP_URL, + auth: FAKE_PAT, }, }, }, @@ -247,17 +212,14 @@ test.describe("MCP GitHub server install flow", () => { await confirmButton.click(); // After deletion the installed list should show the empty state - await expect( - page.getByTestId("mcp-installed-empty"), - ).toBeVisible({ timeout: 10_000 }); + await expect(page.getByTestId("mcp-installed-empty")).toBeVisible({ + timeout: 10_000, + }); // Verify via the settings API that the server was removed - const settingsResp = await page.request.get( - `${BACKEND_URL}/api/settings`, - { - headers: { "X-Session-API-Key": SESSION_API_KEY }, - }, - ); + const settingsResp = await page.request.get(`${BACKEND_URL}/api/settings`, { + headers: { "X-Session-API-Key": SESSION_API_KEY }, + }); expect(settingsResp.ok()).toBe(true); const settings = await settingsResp.json(); const mcpConfig = settings?.agent_settings?.mcp_config;