From e409c57d904a98dd5e69d686a75629d749d8cf13 Mon Sep 17 00:00:00 2001 From: Ashwin Bhat Date: Mon, 2 Jun 2025 09:03:45 -0700 Subject: [PATCH] feat: add mcp_config input that merges with existing mcp server (#96) * feat: add mcp_config input that merges with existing mcp server - Add mcp_config input parameter to action.yml - Modify prepareMcpConfig() to accept and merge additional config - Provided config overrides built-in servers in case of naming collisions - Pass MCP_CONFIG environment variable from action to prepare step Co-authored-by: ashwin-ant * refactor: improve MCP config validation and merging logic - Add JSON validation to ensure parsed config is an object - Simplify merge logic with explicit mcpServers merging - Enhance error logging with config preview for debugging Co-authored-by: ashwin-ant * refactor: improve MCP config logging per review feedback - Remove configPreview from error logging to avoid cluttering output - Add informational log when merging MCP server configurations - Simplify error message for failed config parsing Co-authored-by: ashwin-ant * test: add comprehensive unit tests for prepareMcpConfig Add tests covering: - Basic functionality with no additional config - Valid JSON merging scenarios - Invalid JSON handling - Empty/null config handling - Server name collision scenarios - Complex nested configurations - Environment variable handling Co-authored-by: ashwin-ant * docs: add mcp_config example with sequential-thinking server - Add mcp_config to inputs table - Add example section showing how to use mcp_config with sequential-thinking MCP server - Include clear explanation that custom servers override built-in servers Co-authored-by: ashwin-ant * readme --------- Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com> Co-authored-by: ashwin-ant --- README.md | 56 ++++++ action.yml | 5 + src/entrypoints/prepare.ts | 2 + src/mcp/install-mcp-server.ts | 37 +++- test/install-mcp-server.test.ts | 344 ++++++++++++++++++++++++++++++++ 5 files changed, 442 insertions(+), 2 deletions(-) create mode 100644 test/install-mcp-server.test.ts diff --git a/README.md b/README.md index d39edca..3b31ae6 100644 --- a/README.md +++ b/README.md @@ -82,6 +82,7 @@ jobs: | `allowed_tools` | Additional tools for Claude to use (the base GitHub tools will always be included) | No | "" | | `disallowed_tools` | Tools that Claude should never use | No | "" | | `custom_instructions` | Additional custom instructions to include in the prompt for Claude | No | "" | +| `mcp_config` | Additional MCP configuration (JSON string) that merges with the built-in GitHub MCP servers | No | "" | | `assignee_trigger` | The assignee username that triggers the action (e.g. @claude). Only used for issue assignment | No | - | | `trigger_phrase` | The trigger phrase to look for in comments, issue/PR bodies, and issue titles | No | `@claude` | @@ -89,6 +90,61 @@ jobs: > **Note**: This action is currently in beta. Features and APIs may change as we continue to improve the integration. +### Using Custom MCP Configuration + +The `mcp_config` input allows you to add custom MCP (Model Context Protocol) servers to extend Claude's capabilities. These servers merge with the built-in GitHub MCP servers. + +#### Basic Example: Adding a Sequential Thinking Server + +```yaml +- uses: anthropics/claude-code-action@beta + with: + anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }} + mcp_config: | + { + "mcpServers": { + "sequential-thinking": { + "command": "npx", + "args": [ + "-y", + "@modelcontextprotocol/server-sequential-thinking" + ] + } + } + } + allowed_tools: "mcp__sequential-thinking__sequentialthinking" # Important: Each MCP tool from your server must be listed here, comma-separated + # ... other inputs +``` + +#### Passing Secrets to MCP Servers + +For MCP servers that require sensitive information like API keys or tokens, use GitHub Secrets in the environment variables: + +```yaml +- uses: anthropics/claude-code-action@beta + with: + anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }} + mcp_config: | + { + "mcpServers": { + "custom-api-server": { + "command": "npx", + "args": ["-y", "@example/api-server"], + "env": { + "API_KEY": "${{ secrets.CUSTOM_API_KEY }}", + "BASE_URL": "https://api.example.com" + } + } + } + } + # ... other inputs +``` + +**Important**: + +- Always use GitHub Secrets (`${{ secrets.SECRET_NAME }}`) for sensitive values like API keys, tokens, or passwords. Never hardcode secrets directly in the workflow file. +- Your custom servers will override any built-in servers with the same name. + ## Examples ### Ways to Tag @claude diff --git a/action.yml b/action.yml index 319a6de..d544f67 100644 --- a/action.yml +++ b/action.yml @@ -39,6 +39,10 @@ inputs: description: "Direct instruction for Claude (bypasses normal trigger detection)" required: false default: "" + mcp_config: + description: "Additional MCP configuration (JSON string) that merges with the built-in GitHub MCP servers" + required: false + default: "" # Auth configuration anthropic_api_key: @@ -92,6 +96,7 @@ runs: ALLOWED_TOOLS: ${{ inputs.allowed_tools }} CUSTOM_INSTRUCTIONS: ${{ inputs.custom_instructions }} DIRECT_PROMPT: ${{ inputs.direct_prompt }} + MCP_CONFIG: ${{ inputs.mcp_config }} OVERRIDE_GITHUB_TOKEN: ${{ inputs.github_token }} GITHUB_RUN_ID: ${{ github.run_id }} diff --git a/src/entrypoints/prepare.ts b/src/entrypoints/prepare.ts index c3e0b38..006a62e 100644 --- a/src/entrypoints/prepare.ts +++ b/src/entrypoints/prepare.ts @@ -84,11 +84,13 @@ async function run() { ); // Step 11: Get MCP configuration + const additionalMcpConfig = process.env.MCP_CONFIG || ""; const mcpConfig = await prepareMcpConfig( githubToken, context.repository.owner, context.repository.repo, branchInfo.currentBranch, + additionalMcpConfig, ); core.setOutput("mcp_config", mcpConfig); } catch (error) { diff --git a/src/mcp/install-mcp-server.ts b/src/mcp/install-mcp-server.ts index 462967d..4a4921b 100644 --- a/src/mcp/install-mcp-server.ts +++ b/src/mcp/install-mcp-server.ts @@ -5,9 +5,10 @@ export async function prepareMcpConfig( owner: string, repo: string, branch: string, + additionalMcpConfig?: string, ): Promise { try { - const mcpConfig = { + const baseMcpConfig = { mcpServers: { github: { command: "docker", @@ -40,7 +41,39 @@ export async function prepareMcpConfig( }, }; - return JSON.stringify(mcpConfig, null, 2); + // Merge with additional MCP config if provided + if (additionalMcpConfig && additionalMcpConfig.trim()) { + try { + const additionalConfig = JSON.parse(additionalMcpConfig); + + // Validate that parsed JSON is an object + if (typeof additionalConfig !== "object" || additionalConfig === null) { + throw new Error("MCP config must be a valid JSON object"); + } + + core.info( + "Merging additional MCP server configuration with built-in servers", + ); + + // Merge configurations with user config overriding built-in servers + const mergedConfig = { + ...baseMcpConfig, + ...additionalConfig, + mcpServers: { + ...baseMcpConfig.mcpServers, + ...additionalConfig.mcpServers, + }, + }; + + return JSON.stringify(mergedConfig, null, 2); + } catch (parseError) { + core.warning( + `Failed to parse additional MCP config: ${parseError}. Using base config only.`, + ); + } + } + + return JSON.stringify(baseMcpConfig, null, 2); } catch (error) { core.setFailed(`Install MCP server failed with error: ${error}`); process.exit(1); diff --git a/test/install-mcp-server.test.ts b/test/install-mcp-server.test.ts new file mode 100644 index 0000000..5a93aa6 --- /dev/null +++ b/test/install-mcp-server.test.ts @@ -0,0 +1,344 @@ +import { describe, test, expect, beforeEach, afterEach, spyOn } from "bun:test"; +import { prepareMcpConfig } from "../src/mcp/install-mcp-server"; +import * as core from "@actions/core"; + +describe("prepareMcpConfig", () => { + let consoleInfoSpy: any; + let consoleWarningSpy: any; + let setFailedSpy: any; + let processExitSpy: any; + + beforeEach(() => { + consoleInfoSpy = spyOn(core, "info").mockImplementation(() => {}); + consoleWarningSpy = spyOn(core, "warning").mockImplementation(() => {}); + setFailedSpy = spyOn(core, "setFailed").mockImplementation(() => {}); + processExitSpy = spyOn(process, "exit").mockImplementation(() => { + throw new Error("Process exit"); + }); + }); + + afterEach(() => { + consoleInfoSpy.mockRestore(); + consoleWarningSpy.mockRestore(); + setFailedSpy.mockRestore(); + processExitSpy.mockRestore(); + }); + + test("should return base config when no additional config is provided", async () => { + const result = await prepareMcpConfig( + "test-token", + "test-owner", + "test-repo", + "test-branch", + ); + + const parsed = JSON.parse(result); + expect(parsed.mcpServers).toBeDefined(); + expect(parsed.mcpServers.github).toBeDefined(); + expect(parsed.mcpServers.github_file_ops).toBeDefined(); + expect(parsed.mcpServers.github.env.GITHUB_PERSONAL_ACCESS_TOKEN).toBe( + "test-token", + ); + expect(parsed.mcpServers.github_file_ops.env.GITHUB_TOKEN).toBe( + "test-token", + ); + expect(parsed.mcpServers.github_file_ops.env.REPO_OWNER).toBe("test-owner"); + expect(parsed.mcpServers.github_file_ops.env.REPO_NAME).toBe("test-repo"); + expect(parsed.mcpServers.github_file_ops.env.BRANCH_NAME).toBe( + "test-branch", + ); + }); + + test("should return base config when additional config is empty string", async () => { + const result = await prepareMcpConfig( + "test-token", + "test-owner", + "test-repo", + "test-branch", + "", + ); + + const parsed = JSON.parse(result); + expect(parsed.mcpServers).toBeDefined(); + expect(parsed.mcpServers.github).toBeDefined(); + expect(parsed.mcpServers.github_file_ops).toBeDefined(); + expect(consoleWarningSpy).not.toHaveBeenCalled(); + }); + + test("should return base config when additional config is whitespace only", async () => { + const result = await prepareMcpConfig( + "test-token", + "test-owner", + "test-repo", + "test-branch", + " \n\t ", + ); + + const parsed = JSON.parse(result); + expect(parsed.mcpServers).toBeDefined(); + expect(parsed.mcpServers.github).toBeDefined(); + expect(parsed.mcpServers.github_file_ops).toBeDefined(); + expect(consoleWarningSpy).not.toHaveBeenCalled(); + }); + + test("should merge valid additional config with base config", async () => { + const additionalConfig = JSON.stringify({ + mcpServers: { + custom_server: { + command: "custom-command", + args: ["arg1", "arg2"], + env: { + CUSTOM_ENV: "custom-value", + }, + }, + }, + }); + + const result = await prepareMcpConfig( + "test-token", + "test-owner", + "test-repo", + "test-branch", + additionalConfig, + ); + + const parsed = JSON.parse(result); + expect(consoleInfoSpy).toHaveBeenCalledWith( + "Merging additional MCP server configuration with built-in servers", + ); + expect(parsed.mcpServers.github).toBeDefined(); + expect(parsed.mcpServers.github_file_ops).toBeDefined(); + expect(parsed.mcpServers.custom_server).toBeDefined(); + expect(parsed.mcpServers.custom_server.command).toBe("custom-command"); + expect(parsed.mcpServers.custom_server.args).toEqual(["arg1", "arg2"]); + expect(parsed.mcpServers.custom_server.env.CUSTOM_ENV).toBe("custom-value"); + }); + + test("should override built-in servers when additional config has same server names", async () => { + const additionalConfig = JSON.stringify({ + mcpServers: { + github: { + command: "overridden-command", + args: ["overridden-arg"], + env: { + OVERRIDDEN_ENV: "overridden-value", + }, + }, + }, + }); + + const result = await prepareMcpConfig( + "test-token", + "test-owner", + "test-repo", + "test-branch", + additionalConfig, + ); + + const parsed = JSON.parse(result); + expect(consoleInfoSpy).toHaveBeenCalledWith( + "Merging additional MCP server configuration with built-in servers", + ); + expect(parsed.mcpServers.github.command).toBe("overridden-command"); + expect(parsed.mcpServers.github.args).toEqual(["overridden-arg"]); + expect(parsed.mcpServers.github.env.OVERRIDDEN_ENV).toBe( + "overridden-value", + ); + expect( + parsed.mcpServers.github.env.GITHUB_PERSONAL_ACCESS_TOKEN, + ).toBeUndefined(); + expect(parsed.mcpServers.github_file_ops).toBeDefined(); + }); + + test("should merge additional root-level properties", async () => { + const additionalConfig = JSON.stringify({ + customProperty: "custom-value", + anotherProperty: { + nested: "value", + }, + mcpServers: { + custom_server: { + command: "custom", + }, + }, + }); + + const result = await prepareMcpConfig( + "test-token", + "test-owner", + "test-repo", + "test-branch", + additionalConfig, + ); + + const parsed = JSON.parse(result); + expect(parsed.customProperty).toBe("custom-value"); + expect(parsed.anotherProperty).toEqual({ nested: "value" }); + expect(parsed.mcpServers.github).toBeDefined(); + expect(parsed.mcpServers.custom_server).toBeDefined(); + }); + + test("should handle invalid JSON gracefully", async () => { + const invalidJson = "{ invalid json }"; + + const result = await prepareMcpConfig( + "test-token", + "test-owner", + "test-repo", + "test-branch", + invalidJson, + ); + + const parsed = JSON.parse(result); + expect(consoleWarningSpy).toHaveBeenCalledWith( + expect.stringContaining("Failed to parse additional MCP config:"), + ); + expect(parsed.mcpServers.github).toBeDefined(); + expect(parsed.mcpServers.github_file_ops).toBeDefined(); + }); + + test("should handle non-object JSON values", async () => { + const nonObjectJson = JSON.stringify("string value"); + + const result = await prepareMcpConfig( + "test-token", + "test-owner", + "test-repo", + "test-branch", + nonObjectJson, + ); + + const parsed = JSON.parse(result); + expect(consoleWarningSpy).toHaveBeenCalledWith( + expect.stringContaining("Failed to parse additional MCP config:"), + ); + expect(consoleWarningSpy).toHaveBeenCalledWith( + expect.stringContaining("MCP config must be a valid JSON object"), + ); + expect(parsed.mcpServers.github).toBeDefined(); + expect(parsed.mcpServers.github_file_ops).toBeDefined(); + }); + + test("should handle null JSON value", async () => { + const nullJson = JSON.stringify(null); + + const result = await prepareMcpConfig( + "test-token", + "test-owner", + "test-repo", + "test-branch", + nullJson, + ); + + const parsed = JSON.parse(result); + expect(consoleWarningSpy).toHaveBeenCalledWith( + expect.stringContaining("Failed to parse additional MCP config:"), + ); + expect(consoleWarningSpy).toHaveBeenCalledWith( + expect.stringContaining("MCP config must be a valid JSON object"), + ); + expect(parsed.mcpServers.github).toBeDefined(); + expect(parsed.mcpServers.github_file_ops).toBeDefined(); + }); + + test("should handle array JSON value", async () => { + const arrayJson = JSON.stringify([1, 2, 3]); + + const result = await prepareMcpConfig( + "test-token", + "test-owner", + "test-repo", + "test-branch", + arrayJson, + ); + + const parsed = JSON.parse(result); + // Arrays are objects in JavaScript, so they pass the object check + // But they'll fail when trying to spread or access mcpServers property + expect(consoleInfoSpy).toHaveBeenCalledWith( + "Merging additional MCP server configuration with built-in servers", + ); + expect(parsed.mcpServers.github).toBeDefined(); + expect(parsed.mcpServers.github_file_ops).toBeDefined(); + // The array will be spread into the config (0: 1, 1: 2, 2: 3) + expect(parsed[0]).toBe(1); + expect(parsed[1]).toBe(2); + expect(parsed[2]).toBe(3); + }); + + test("should merge complex nested configurations", async () => { + const additionalConfig = JSON.stringify({ + mcpServers: { + server1: { + command: "cmd1", + env: { KEY1: "value1" }, + }, + server2: { + command: "cmd2", + env: { KEY2: "value2" }, + }, + github_file_ops: { + command: "overridden", + env: { CUSTOM: "value" }, + }, + }, + otherConfig: { + nested: { + deeply: "value", + }, + }, + }); + + const result = await prepareMcpConfig( + "test-token", + "test-owner", + "test-repo", + "test-branch", + additionalConfig, + ); + + const parsed = JSON.parse(result); + expect(parsed.mcpServers.server1).toBeDefined(); + expect(parsed.mcpServers.server2).toBeDefined(); + expect(parsed.mcpServers.github).toBeDefined(); + expect(parsed.mcpServers.github_file_ops.command).toBe("overridden"); + expect(parsed.mcpServers.github_file_ops.env.CUSTOM).toBe("value"); + expect(parsed.otherConfig.nested.deeply).toBe("value"); + }); + + test("should preserve GITHUB_ACTION_PATH in file_ops server args", async () => { + const oldEnv = process.env.GITHUB_ACTION_PATH; + process.env.GITHUB_ACTION_PATH = "/test/action/path"; + + const result = await prepareMcpConfig( + "test-token", + "test-owner", + "test-repo", + "test-branch", + ); + + const parsed = JSON.parse(result); + expect(parsed.mcpServers.github_file_ops.args[1]).toBe( + "/test/action/path/src/mcp/github-file-ops-server.ts", + ); + + process.env.GITHUB_ACTION_PATH = oldEnv; + }); + + test("should use process.cwd() when GITHUB_WORKSPACE is not set", async () => { + const oldEnv = process.env.GITHUB_WORKSPACE; + delete process.env.GITHUB_WORKSPACE; + + const result = await prepareMcpConfig( + "test-token", + "test-owner", + "test-repo", + "test-branch", + ); + + const parsed = JSON.parse(result); + expect(parsed.mcpServers.github_file_ops.env.REPO_DIR).toBe(process.cwd()); + + process.env.GITHUB_WORKSPACE = oldEnv; + }); +});