mirror of
https://github.com/anthropics/claude-code-action.git
synced 2026-01-23 06:54:13 +08:00
Simplify MCP configuration to use multiple --mcp-config flags
- Remove MCP config merging logic from prepareMcpConfig - Update agent and tag modes to pass multiple --mcp-config flags - Let Claude handle config merging natively through multiple flags - Fix TypeScript errors in test file This approach is cleaner and relies on Claude's built-in support for multiple --mcp-config flags instead of manual JSON merging.
This commit is contained in:
@@ -10,7 +10,6 @@ type PrepareConfigParams = {
|
|||||||
repo: string;
|
repo: string;
|
||||||
branch: string;
|
branch: string;
|
||||||
baseBranch: string;
|
baseBranch: string;
|
||||||
additionalMcpConfig?: string;
|
|
||||||
claudeCommentId?: string;
|
claudeCommentId?: string;
|
||||||
allowedTools: string[];
|
allowedTools: string[];
|
||||||
context: GitHubContext;
|
context: GitHubContext;
|
||||||
@@ -57,7 +56,6 @@ export async function prepareMcpConfig(
|
|||||||
repo,
|
repo,
|
||||||
branch,
|
branch,
|
||||||
baseBranch,
|
baseBranch,
|
||||||
additionalMcpConfig,
|
|
||||||
claudeCommentId,
|
claudeCommentId,
|
||||||
allowedTools,
|
allowedTools,
|
||||||
context,
|
context,
|
||||||
@@ -193,38 +191,8 @@ export async function prepareMcpConfig(
|
|||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
// Merge with additional MCP config if provided
|
// Return only our GitHub servers config
|
||||||
if (additionalMcpConfig && additionalMcpConfig.trim()) {
|
// User's config will be passed as separate --mcp-config flags
|
||||||
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);
|
return JSON.stringify(baseMcpConfig, null, 2);
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
core.setFailed(`Install MCP server failed with error: ${error}`);
|
core.setFailed(`Install MCP server failed with error: ${error}`);
|
||||||
|
|||||||
@@ -65,24 +65,37 @@ export const agentMode: Mode = {
|
|||||||
const currentBranch =
|
const currentBranch =
|
||||||
process.env.GITHUB_HEAD_REF || process.env.GITHUB_REF_NAME || "main";
|
process.env.GITHUB_HEAD_REF || process.env.GITHUB_REF_NAME || "main";
|
||||||
|
|
||||||
// Get MCP configuration with GitHub servers when requested
|
// Get our GitHub MCP servers config
|
||||||
const additionalMcpConfig = process.env.MCP_CONFIG || "";
|
const ourMcpConfig = await prepareMcpConfig({
|
||||||
const mcpConfig = await prepareMcpConfig({
|
|
||||||
githubToken,
|
githubToken,
|
||||||
owner: context.repository.owner,
|
owner: context.repository.owner,
|
||||||
repo: context.repository.repo,
|
repo: context.repository.repo,
|
||||||
branch: currentBranch,
|
branch: currentBranch,
|
||||||
baseBranch: context.inputs.baseBranch || "main",
|
baseBranch: context.inputs.baseBranch || "main",
|
||||||
additionalMcpConfig,
|
|
||||||
claudeCommentId: undefined, // No tracking comment in agent mode
|
claudeCommentId: undefined, // No tracking comment in agent mode
|
||||||
allowedTools,
|
allowedTools,
|
||||||
context,
|
context,
|
||||||
});
|
});
|
||||||
|
|
||||||
// Build final claude_args
|
// Build final claude_args with multiple --mcp-config flags
|
||||||
const escapedMcpConfig = mcpConfig.replace(/'/g, "'\\''");
|
let claudeArgs = "";
|
||||||
const claudeArgs =
|
|
||||||
`--mcp-config '${escapedMcpConfig}' ${userClaudeArgs}`.trim();
|
// Add our GitHub servers config if we have any
|
||||||
|
const ourConfig = JSON.parse(ourMcpConfig);
|
||||||
|
if (ourConfig.mcpServers && Object.keys(ourConfig.mcpServers).length > 0) {
|
||||||
|
const escapedOurConfig = ourMcpConfig.replace(/'/g, "'\\''");
|
||||||
|
claudeArgs = `--mcp-config '${escapedOurConfig}'`;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Add user's MCP_CONFIG env var as separate --mcp-config
|
||||||
|
const userMcpConfig = process.env.MCP_CONFIG;
|
||||||
|
if (userMcpConfig?.trim()) {
|
||||||
|
const escapedUserConfig = userMcpConfig.replace(/'/g, "'\\''");
|
||||||
|
claudeArgs = `${claudeArgs} --mcp-config '${escapedUserConfig}'`.trim();
|
||||||
|
}
|
||||||
|
|
||||||
|
// Append user's claude_args (which may have more --mcp-config flags)
|
||||||
|
claudeArgs = `${claudeArgs} ${userClaudeArgs}`.trim();
|
||||||
|
|
||||||
core.setOutput("claude_args", claudeArgs);
|
core.setOutput("claude_args", claudeArgs);
|
||||||
|
|
||||||
@@ -93,7 +106,7 @@ export const agentMode: Mode = {
|
|||||||
currentBranch,
|
currentBranch,
|
||||||
claudeBranch: undefined,
|
claudeBranch: undefined,
|
||||||
},
|
},
|
||||||
mcpConfig,
|
mcpConfig: ourMcpConfig,
|
||||||
};
|
};
|
||||||
},
|
},
|
||||||
|
|
||||||
|
|||||||
@@ -100,15 +100,13 @@ export const tagMode: Mode = {
|
|||||||
|
|
||||||
await createPrompt(tagMode, modeContext, githubData, context);
|
await createPrompt(tagMode, modeContext, githubData, context);
|
||||||
|
|
||||||
// Get MCP configuration
|
// Get our GitHub MCP servers configuration
|
||||||
const additionalMcpConfig = process.env.MCP_CONFIG || "";
|
const ourMcpConfig = await prepareMcpConfig({
|
||||||
const mcpConfig = await prepareMcpConfig({
|
|
||||||
githubToken,
|
githubToken,
|
||||||
owner: context.repository.owner,
|
owner: context.repository.owner,
|
||||||
repo: context.repository.repo,
|
repo: context.repository.repo,
|
||||||
branch: branchInfo.claudeBranch || branchInfo.currentBranch,
|
branch: branchInfo.claudeBranch || branchInfo.currentBranch,
|
||||||
baseBranch: branchInfo.baseBranch,
|
baseBranch: branchInfo.baseBranch,
|
||||||
additionalMcpConfig,
|
|
||||||
claudeCommentId: commentId.toString(),
|
claudeCommentId: commentId.toString(),
|
||||||
allowedTools: [],
|
allowedTools: [],
|
||||||
context,
|
context,
|
||||||
@@ -150,14 +148,26 @@ export const tagMode: Mode = {
|
|||||||
|
|
||||||
const userClaudeArgs = process.env.CLAUDE_ARGS || "";
|
const userClaudeArgs = process.env.CLAUDE_ARGS || "";
|
||||||
|
|
||||||
// Build complete claude_args with MCP config (as JSON string), tools, and user args
|
// Build complete claude_args with multiple --mcp-config flags
|
||||||
// Note: Once Claude supports multiple --mcp-config flags, we can pass as file path
|
let claudeArgs = "";
|
||||||
// Escape single quotes in JSON to prevent shell injection
|
|
||||||
const escapedMcpConfig = mcpConfig.replace(/'/g, "'\\''");
|
// Add our GitHub servers config
|
||||||
let claudeArgs = `--mcp-config '${escapedMcpConfig}' `;
|
const escapedOurConfig = ourMcpConfig.replace(/'/g, "'\\''");
|
||||||
|
claudeArgs = `--mcp-config '${escapedOurConfig}'`;
|
||||||
|
|
||||||
|
// Add user's MCP_CONFIG env var as separate --mcp-config
|
||||||
|
const userMcpConfig = process.env.MCP_CONFIG;
|
||||||
|
if (userMcpConfig?.trim()) {
|
||||||
|
const escapedUserConfig = userMcpConfig.replace(/'/g, "'\\''");
|
||||||
|
claudeArgs = `${claudeArgs} --mcp-config '${escapedUserConfig}'`;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Add required tools for tag mode
|
||||||
claudeArgs += ` --allowedTools "${tagModeTools.join(",")}"`;
|
claudeArgs += ` --allowedTools "${tagModeTools.join(",")}"`;
|
||||||
|
|
||||||
|
// Append user's claude_args (which may have more --mcp-config flags)
|
||||||
if (userClaudeArgs) {
|
if (userClaudeArgs) {
|
||||||
claudeArgs += userClaudeArgs;
|
claudeArgs += ` ${userClaudeArgs}`;
|
||||||
}
|
}
|
||||||
|
|
||||||
core.setOutput("claude_args", claudeArgs.trim());
|
core.setOutput("claude_args", claudeArgs.trim());
|
||||||
@@ -165,7 +175,7 @@ export const tagMode: Mode = {
|
|||||||
return {
|
return {
|
||||||
commentId,
|
commentId,
|
||||||
branchInfo,
|
branchInfo,
|
||||||
mcpConfig,
|
mcpConfig: ourMcpConfig,
|
||||||
};
|
};
|
||||||
},
|
},
|
||||||
|
|
||||||
|
|||||||
@@ -50,14 +50,6 @@ describe("prepareMcpConfig", () => {
|
|||||||
},
|
},
|
||||||
};
|
};
|
||||||
|
|
||||||
const mockPRContextWithSigning: ParsedGitHubContext = {
|
|
||||||
...mockPRContext,
|
|
||||||
inputs: {
|
|
||||||
...mockPRContext.inputs,
|
|
||||||
useCommitSigning: true,
|
|
||||||
},
|
|
||||||
};
|
|
||||||
|
|
||||||
beforeEach(() => {
|
beforeEach(() => {
|
||||||
consoleInfoSpy = spyOn(core, "info").mockImplementation(() => {});
|
consoleInfoSpy = spyOn(core, "info").mockImplementation(() => {});
|
||||||
consoleWarningSpy = spyOn(core, "warning").mockImplementation(() => {});
|
consoleWarningSpy = spyOn(core, "warning").mockImplementation(() => {});
|
||||||
@@ -98,19 +90,9 @@ describe("prepareMcpConfig", () => {
|
|||||||
expect(parsed.mcpServers.github_comment.env.GITHUB_TOKEN).toBe(
|
expect(parsed.mcpServers.github_comment.env.GITHUB_TOKEN).toBe(
|
||||||
"test-token",
|
"test-token",
|
||||||
);
|
);
|
||||||
expect(parsed.mcpServers.github_comment.env.REPO_OWNER).toBe("test-owner");
|
|
||||||
expect(parsed.mcpServers.github_comment.env.REPO_NAME).toBe("test-repo");
|
|
||||||
});
|
});
|
||||||
|
|
||||||
test("should return file ops server when commit signing is enabled", async () => {
|
test("should include file ops server when commit signing is enabled", async () => {
|
||||||
const contextWithSigning = {
|
|
||||||
...mockContext,
|
|
||||||
inputs: {
|
|
||||||
...mockContext.inputs,
|
|
||||||
useCommitSigning: true,
|
|
||||||
},
|
|
||||||
};
|
|
||||||
|
|
||||||
const result = await prepareMcpConfig({
|
const result = await prepareMcpConfig({
|
||||||
githubToken: "test-token",
|
githubToken: "test-token",
|
||||||
owner: "test-owner",
|
owner: "test-owner",
|
||||||
@@ -118,19 +100,16 @@ describe("prepareMcpConfig", () => {
|
|||||||
branch: "test-branch",
|
branch: "test-branch",
|
||||||
baseBranch: "main",
|
baseBranch: "main",
|
||||||
allowedTools: [],
|
allowedTools: [],
|
||||||
context: contextWithSigning,
|
context: mockContextWithSigning,
|
||||||
});
|
});
|
||||||
|
|
||||||
const parsed = JSON.parse(result);
|
const parsed = JSON.parse(result);
|
||||||
expect(parsed.mcpServers).toBeDefined();
|
expect(parsed.mcpServers).toBeDefined();
|
||||||
expect(parsed.mcpServers.github).not.toBeDefined();
|
expect(parsed.mcpServers.github).not.toBeDefined();
|
||||||
expect(parsed.mcpServers.github_comment).toBeDefined();
|
|
||||||
expect(parsed.mcpServers.github_file_ops).toBeDefined();
|
expect(parsed.mcpServers.github_file_ops).toBeDefined();
|
||||||
expect(parsed.mcpServers.github_file_ops.env.GITHUB_TOKEN).toBe(
|
expect(parsed.mcpServers.github_file_ops.env.GITHUB_TOKEN).toBe(
|
||||||
"test-token",
|
"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(
|
expect(parsed.mcpServers.github_file_ops.env.BRANCH_NAME).toBe(
|
||||||
"test-branch",
|
"test-branch",
|
||||||
);
|
);
|
||||||
@@ -143,49 +122,37 @@ describe("prepareMcpConfig", () => {
|
|||||||
repo: "test-repo",
|
repo: "test-repo",
|
||||||
branch: "test-branch",
|
branch: "test-branch",
|
||||||
baseBranch: "main",
|
baseBranch: "main",
|
||||||
allowedTools: [
|
allowedTools: ["mcp__github__create_issue", "mcp__github__create_pr"],
|
||||||
"mcp__github__create_issue",
|
|
||||||
"mcp__github_file_ops__commit_files",
|
|
||||||
],
|
|
||||||
context: mockContext,
|
context: mockContext,
|
||||||
});
|
});
|
||||||
|
|
||||||
const parsed = JSON.parse(result);
|
const parsed = JSON.parse(result);
|
||||||
expect(parsed.mcpServers).toBeDefined();
|
expect(parsed.mcpServers).toBeDefined();
|
||||||
expect(parsed.mcpServers.github).toBeDefined();
|
expect(parsed.mcpServers.github).toBeDefined();
|
||||||
expect(parsed.mcpServers.github_comment).toBeDefined();
|
expect(parsed.mcpServers.github.command).toBe("docker");
|
||||||
expect(parsed.mcpServers.github_file_ops).not.toBeDefined();
|
|
||||||
expect(parsed.mcpServers.github.env.GITHUB_PERSONAL_ACCESS_TOKEN).toBe(
|
expect(parsed.mcpServers.github.env.GITHUB_PERSONAL_ACCESS_TOKEN).toBe(
|
||||||
"test-token",
|
"test-token",
|
||||||
);
|
);
|
||||||
});
|
});
|
||||||
|
|
||||||
test("should not include github MCP server when only file_ops tools are allowed", async () => {
|
test("should include inline comment server for PRs when tools are allowed", async () => {
|
||||||
const contextWithSigning = {
|
|
||||||
...mockContext,
|
|
||||||
inputs: {
|
|
||||||
...mockContext.inputs,
|
|
||||||
useCommitSigning: true,
|
|
||||||
},
|
|
||||||
};
|
|
||||||
|
|
||||||
const result = await prepareMcpConfig({
|
const result = await prepareMcpConfig({
|
||||||
githubToken: "test-token",
|
githubToken: "test-token",
|
||||||
owner: "test-owner",
|
owner: "test-owner",
|
||||||
repo: "test-repo",
|
repo: "test-repo",
|
||||||
branch: "test-branch",
|
branch: "test-branch",
|
||||||
baseBranch: "main",
|
baseBranch: "main",
|
||||||
allowedTools: [
|
allowedTools: ["mcp__github_inline_comment__create_inline_comment"],
|
||||||
"mcp__github_file_ops__commit_files",
|
context: mockPRContext,
|
||||||
"mcp__github_file_ops__update_claude_comment",
|
|
||||||
],
|
|
||||||
context: contextWithSigning,
|
|
||||||
});
|
});
|
||||||
|
|
||||||
const parsed = JSON.parse(result);
|
const parsed = JSON.parse(result);
|
||||||
expect(parsed.mcpServers).toBeDefined();
|
expect(parsed.mcpServers).toBeDefined();
|
||||||
expect(parsed.mcpServers.github).not.toBeDefined();
|
expect(parsed.mcpServers.github_inline_comment).toBeDefined();
|
||||||
expect(parsed.mcpServers.github_file_ops).toBeDefined();
|
expect(parsed.mcpServers.github_inline_comment.env.GITHUB_TOKEN).toBe(
|
||||||
|
"test-token",
|
||||||
|
);
|
||||||
|
expect(parsed.mcpServers.github_inline_comment.env.PR_NUMBER).toBe("456");
|
||||||
});
|
});
|
||||||
|
|
||||||
test("should include comment server when no GitHub tools are allowed and signing disabled", async () => {
|
test("should include comment server when no GitHub tools are allowed and signing disabled", async () => {
|
||||||
@@ -195,7 +162,7 @@ describe("prepareMcpConfig", () => {
|
|||||||
repo: "test-repo",
|
repo: "test-repo",
|
||||||
branch: "test-branch",
|
branch: "test-branch",
|
||||||
baseBranch: "main",
|
baseBranch: "main",
|
||||||
allowedTools: ["Edit", "Read", "Write"],
|
allowedTools: [],
|
||||||
context: mockContext,
|
context: mockContext,
|
||||||
});
|
});
|
||||||
|
|
||||||
@@ -206,301 +173,7 @@ describe("prepareMcpConfig", () => {
|
|||||||
expect(parsed.mcpServers.github_comment).toBeDefined();
|
expect(parsed.mcpServers.github_comment).toBeDefined();
|
||||||
});
|
});
|
||||||
|
|
||||||
test("should return base config when additional config is empty string", async () => {
|
test("should set GITHUB_ACTION_PATH correctly", async () => {
|
||||||
const result = await prepareMcpConfig({
|
|
||||||
githubToken: "test-token",
|
|
||||||
owner: "test-owner",
|
|
||||||
repo: "test-repo",
|
|
||||||
branch: "test-branch",
|
|
||||||
baseBranch: "main",
|
|
||||||
additionalMcpConfig: "",
|
|
||||||
allowedTools: [],
|
|
||||||
context: mockContext,
|
|
||||||
});
|
|
||||||
|
|
||||||
const parsed = JSON.parse(result);
|
|
||||||
expect(parsed.mcpServers).toBeDefined();
|
|
||||||
expect(parsed.mcpServers.github).not.toBeDefined();
|
|
||||||
expect(parsed.mcpServers.github_comment).toBeDefined();
|
|
||||||
expect(consoleWarningSpy).not.toHaveBeenCalled();
|
|
||||||
});
|
|
||||||
|
|
||||||
test("should return base config when additional config is whitespace only", async () => {
|
|
||||||
const result = await prepareMcpConfig({
|
|
||||||
githubToken: "test-token",
|
|
||||||
owner: "test-owner",
|
|
||||||
repo: "test-repo",
|
|
||||||
branch: "test-branch",
|
|
||||||
baseBranch: "main",
|
|
||||||
additionalMcpConfig: " \n\t ",
|
|
||||||
allowedTools: [],
|
|
||||||
context: mockContext,
|
|
||||||
});
|
|
||||||
|
|
||||||
const parsed = JSON.parse(result);
|
|
||||||
expect(parsed.mcpServers).toBeDefined();
|
|
||||||
expect(parsed.mcpServers.github).not.toBeDefined();
|
|
||||||
expect(parsed.mcpServers.github_comment).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({
|
|
||||||
githubToken: "test-token",
|
|
||||||
owner: "test-owner",
|
|
||||||
repo: "test-repo",
|
|
||||||
branch: "test-branch",
|
|
||||||
baseBranch: "main",
|
|
||||||
additionalMcpConfig: additionalConfig,
|
|
||||||
allowedTools: [
|
|
||||||
"mcp__github__create_issue",
|
|
||||||
"mcp__github_file_ops__commit_files",
|
|
||||||
],
|
|
||||||
context: mockContextWithSigning,
|
|
||||||
});
|
|
||||||
|
|
||||||
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({
|
|
||||||
githubToken: "test-token",
|
|
||||||
owner: "test-owner",
|
|
||||||
repo: "test-repo",
|
|
||||||
branch: "test-branch",
|
|
||||||
baseBranch: "main",
|
|
||||||
additionalMcpConfig: additionalConfig,
|
|
||||||
allowedTools: [
|
|
||||||
"mcp__github__create_issue",
|
|
||||||
"mcp__github_file_ops__commit_files",
|
|
||||||
],
|
|
||||||
context: mockContextWithSigning,
|
|
||||||
});
|
|
||||||
|
|
||||||
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({
|
|
||||||
githubToken: "test-token",
|
|
||||||
owner: "test-owner",
|
|
||||||
repo: "test-repo",
|
|
||||||
branch: "test-branch",
|
|
||||||
baseBranch: "main",
|
|
||||||
additionalMcpConfig: additionalConfig,
|
|
||||||
allowedTools: [],
|
|
||||||
context: mockContextWithSigning,
|
|
||||||
});
|
|
||||||
|
|
||||||
const parsed = JSON.parse(result);
|
|
||||||
expect(parsed.customProperty).toBe("custom-value");
|
|
||||||
expect(parsed.anotherProperty).toEqual({ nested: "value" });
|
|
||||||
expect(parsed.mcpServers.github).not.toBeDefined();
|
|
||||||
expect(parsed.mcpServers.custom_server).toBeDefined();
|
|
||||||
});
|
|
||||||
|
|
||||||
test("should handle invalid JSON gracefully", async () => {
|
|
||||||
const invalidJson = "{ invalid json }";
|
|
||||||
|
|
||||||
const result = await prepareMcpConfig({
|
|
||||||
githubToken: "test-token",
|
|
||||||
owner: "test-owner",
|
|
||||||
repo: "test-repo",
|
|
||||||
branch: "test-branch",
|
|
||||||
baseBranch: "main",
|
|
||||||
additionalMcpConfig: invalidJson,
|
|
||||||
allowedTools: [],
|
|
||||||
context: mockContextWithSigning,
|
|
||||||
});
|
|
||||||
|
|
||||||
const parsed = JSON.parse(result);
|
|
||||||
expect(consoleWarningSpy).toHaveBeenCalledWith(
|
|
||||||
expect.stringContaining("Failed to parse additional MCP config:"),
|
|
||||||
);
|
|
||||||
expect(parsed.mcpServers.github).not.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({
|
|
||||||
githubToken: "test-token",
|
|
||||||
owner: "test-owner",
|
|
||||||
repo: "test-repo",
|
|
||||||
branch: "test-branch",
|
|
||||||
baseBranch: "main",
|
|
||||||
additionalMcpConfig: nonObjectJson,
|
|
||||||
allowedTools: [],
|
|
||||||
context: mockContextWithSigning,
|
|
||||||
});
|
|
||||||
|
|
||||||
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).not.toBeDefined();
|
|
||||||
expect(parsed.mcpServers.github_file_ops).toBeDefined();
|
|
||||||
});
|
|
||||||
|
|
||||||
test("should handle null JSON value", async () => {
|
|
||||||
const nullJson = JSON.stringify(null);
|
|
||||||
|
|
||||||
const result = await prepareMcpConfig({
|
|
||||||
githubToken: "test-token",
|
|
||||||
owner: "test-owner",
|
|
||||||
repo: "test-repo",
|
|
||||||
branch: "test-branch",
|
|
||||||
baseBranch: "main",
|
|
||||||
additionalMcpConfig: nullJson,
|
|
||||||
allowedTools: [],
|
|
||||||
context: mockContextWithSigning,
|
|
||||||
});
|
|
||||||
|
|
||||||
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).not.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({
|
|
||||||
githubToken: "test-token",
|
|
||||||
owner: "test-owner",
|
|
||||||
repo: "test-repo",
|
|
||||||
branch: "test-branch",
|
|
||||||
baseBranch: "main",
|
|
||||||
additionalMcpConfig: arrayJson,
|
|
||||||
allowedTools: [],
|
|
||||||
context: mockContextWithSigning,
|
|
||||||
});
|
|
||||||
|
|
||||||
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).not.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({
|
|
||||||
githubToken: "test-token",
|
|
||||||
owner: "test-owner",
|
|
||||||
repo: "test-repo",
|
|
||||||
branch: "test-branch",
|
|
||||||
baseBranch: "main",
|
|
||||||
additionalMcpConfig: additionalConfig,
|
|
||||||
allowedTools: [],
|
|
||||||
context: mockContextWithSigning,
|
|
||||||
});
|
|
||||||
|
|
||||||
const parsed = JSON.parse(result);
|
|
||||||
expect(parsed.mcpServers.server1).toBeDefined();
|
|
||||||
expect(parsed.mcpServers.server2).toBeDefined();
|
|
||||||
expect(parsed.mcpServers.github).not.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";
|
process.env.GITHUB_ACTION_PATH = "/test/action/path";
|
||||||
|
|
||||||
const result = await prepareMcpConfig({
|
const result = await prepareMcpConfig({
|
||||||
@@ -514,15 +187,12 @@ describe("prepareMcpConfig", () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
const parsed = JSON.parse(result);
|
const parsed = JSON.parse(result);
|
||||||
expect(parsed.mcpServers.github_file_ops.args[1]).toBe(
|
expect(parsed.mcpServers.github_file_ops.args).toContain(
|
||||||
"/test/action/path/src/mcp/github-file-ops-server.ts",
|
"/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 () => {
|
test("should use current working directory when GITHUB_WORKSPACE is not set", async () => {
|
||||||
const oldEnv = process.env.GITHUB_WORKSPACE;
|
|
||||||
delete process.env.GITHUB_WORKSPACE;
|
delete process.env.GITHUB_WORKSPACE;
|
||||||
|
|
||||||
const result = await prepareMcpConfig({
|
const result = await prepareMcpConfig({
|
||||||
@@ -537,22 +207,11 @@ describe("prepareMcpConfig", () => {
|
|||||||
|
|
||||||
const parsed = JSON.parse(result);
|
const parsed = JSON.parse(result);
|
||||||
expect(parsed.mcpServers.github_file_ops.env.REPO_DIR).toBe(process.cwd());
|
expect(parsed.mcpServers.github_file_ops.env.REPO_DIR).toBe(process.cwd());
|
||||||
|
|
||||||
process.env.GITHUB_WORKSPACE = oldEnv;
|
|
||||||
});
|
});
|
||||||
|
|
||||||
test("should include github_ci server when context.isPR is true and workflow token is present", async () => {
|
test("should include CI server when context.isPR is true and DEFAULT_WORKFLOW_TOKEN exists", async () => {
|
||||||
const oldEnv = process.env.DEFAULT_WORKFLOW_TOKEN;
|
|
||||||
process.env.DEFAULT_WORKFLOW_TOKEN = "workflow-token";
|
process.env.DEFAULT_WORKFLOW_TOKEN = "workflow-token";
|
||||||
|
|
||||||
const contextWithPermissions = {
|
|
||||||
...mockPRContext,
|
|
||||||
inputs: {
|
|
||||||
...mockPRContext.inputs,
|
|
||||||
useCommitSigning: true,
|
|
||||||
},
|
|
||||||
};
|
|
||||||
|
|
||||||
const result = await prepareMcpConfig({
|
const result = await prepareMcpConfig({
|
||||||
githubToken: "test-token",
|
githubToken: "test-token",
|
||||||
owner: "test-owner",
|
owner: "test-owner",
|
||||||
@@ -560,16 +219,15 @@ describe("prepareMcpConfig", () => {
|
|||||||
branch: "test-branch",
|
branch: "test-branch",
|
||||||
baseBranch: "main",
|
baseBranch: "main",
|
||||||
allowedTools: [],
|
allowedTools: [],
|
||||||
context: contextWithPermissions,
|
context: mockPRContext,
|
||||||
});
|
});
|
||||||
|
|
||||||
const parsed = JSON.parse(result);
|
const parsed = JSON.parse(result);
|
||||||
expect(parsed.mcpServers.github_ci).toBeDefined();
|
expect(parsed.mcpServers.github_ci).toBeDefined();
|
||||||
expect(parsed.mcpServers.github_ci.env.GITHUB_TOKEN).toBe("workflow-token");
|
expect(parsed.mcpServers.github_ci.env.GITHUB_TOKEN).toBe("workflow-token");
|
||||||
expect(parsed.mcpServers.github_ci.env.PR_NUMBER).toBe("456");
|
expect(parsed.mcpServers.github_ci.env.PR_NUMBER).toBe("456");
|
||||||
expect(parsed.mcpServers.github_file_ops).toBeDefined();
|
|
||||||
|
|
||||||
process.env.DEFAULT_WORKFLOW_TOKEN = oldEnv;
|
delete process.env.DEFAULT_WORKFLOW_TOKEN;
|
||||||
});
|
});
|
||||||
|
|
||||||
test("should not include github_ci server when context.isPR is false", async () => {
|
test("should not include github_ci server when context.isPR is false", async () => {
|
||||||
@@ -580,16 +238,14 @@ describe("prepareMcpConfig", () => {
|
|||||||
branch: "test-branch",
|
branch: "test-branch",
|
||||||
baseBranch: "main",
|
baseBranch: "main",
|
||||||
allowedTools: [],
|
allowedTools: [],
|
||||||
context: mockContextWithSigning,
|
context: mockContext,
|
||||||
});
|
});
|
||||||
|
|
||||||
const parsed = JSON.parse(result);
|
const parsed = JSON.parse(result);
|
||||||
expect(parsed.mcpServers.github_ci).not.toBeDefined();
|
expect(parsed.mcpServers.github_ci).not.toBeDefined();
|
||||||
expect(parsed.mcpServers.github_file_ops).toBeDefined();
|
|
||||||
});
|
});
|
||||||
|
|
||||||
test("should not include github_ci server when workflow token is not present", async () => {
|
test("should not include github_ci server when DEFAULT_WORKFLOW_TOKEN is missing", async () => {
|
||||||
const oldTokenEnv = process.env.DEFAULT_WORKFLOW_TOKEN;
|
|
||||||
delete process.env.DEFAULT_WORKFLOW_TOKEN;
|
delete process.env.DEFAULT_WORKFLOW_TOKEN;
|
||||||
|
|
||||||
const result = await prepareMcpConfig({
|
const result = await prepareMcpConfig({
|
||||||
@@ -599,73 +255,10 @@ describe("prepareMcpConfig", () => {
|
|||||||
branch: "test-branch",
|
branch: "test-branch",
|
||||||
baseBranch: "main",
|
baseBranch: "main",
|
||||||
allowedTools: [],
|
allowedTools: [],
|
||||||
context: mockPRContextWithSigning,
|
context: mockPRContext,
|
||||||
});
|
});
|
||||||
|
|
||||||
const parsed = JSON.parse(result);
|
const parsed = JSON.parse(result);
|
||||||
expect(parsed.mcpServers.github_ci).not.toBeDefined();
|
expect(parsed.mcpServers.github_ci).not.toBeDefined();
|
||||||
expect(parsed.mcpServers.github_file_ops).toBeDefined();
|
|
||||||
|
|
||||||
process.env.DEFAULT_WORKFLOW_TOKEN = oldTokenEnv;
|
|
||||||
});
|
|
||||||
|
|
||||||
test("should include github_ci server when workflow token is present for PR context", async () => {
|
|
||||||
const oldTokenEnv = process.env.DEFAULT_WORKFLOW_TOKEN;
|
|
||||||
process.env.DEFAULT_WORKFLOW_TOKEN = "workflow-token";
|
|
||||||
|
|
||||||
const contextWithPermissions = {
|
|
||||||
...mockPRContext,
|
|
||||||
inputs: {
|
|
||||||
...mockPRContext.inputs,
|
|
||||||
},
|
|
||||||
};
|
|
||||||
|
|
||||||
const result = await prepareMcpConfig({
|
|
||||||
githubToken: "test-token",
|
|
||||||
owner: "test-owner",
|
|
||||||
repo: "test-repo",
|
|
||||||
branch: "test-branch",
|
|
||||||
baseBranch: "main",
|
|
||||||
allowedTools: [],
|
|
||||||
context: contextWithPermissions,
|
|
||||||
});
|
|
||||||
|
|
||||||
const parsed = JSON.parse(result);
|
|
||||||
expect(parsed.mcpServers.github_ci).toBeDefined();
|
|
||||||
expect(parsed.mcpServers.github_ci.env.GITHUB_TOKEN).toBe("workflow-token");
|
|
||||||
|
|
||||||
process.env.DEFAULT_WORKFLOW_TOKEN = oldTokenEnv;
|
|
||||||
});
|
|
||||||
|
|
||||||
test("should warn when workflow token lacks actions:read permission", async () => {
|
|
||||||
const oldTokenEnv = process.env.DEFAULT_WORKFLOW_TOKEN;
|
|
||||||
process.env.DEFAULT_WORKFLOW_TOKEN = "invalid-token";
|
|
||||||
|
|
||||||
const contextWithPermissions = {
|
|
||||||
...mockPRContext,
|
|
||||||
inputs: {
|
|
||||||
...mockPRContext.inputs,
|
|
||||||
},
|
|
||||||
};
|
|
||||||
|
|
||||||
const result = await prepareMcpConfig({
|
|
||||||
githubToken: "test-token",
|
|
||||||
owner: "test-owner",
|
|
||||||
repo: "test-repo",
|
|
||||||
branch: "test-branch",
|
|
||||||
baseBranch: "main",
|
|
||||||
allowedTools: [],
|
|
||||||
context: contextWithPermissions,
|
|
||||||
});
|
|
||||||
|
|
||||||
const parsed = JSON.parse(result);
|
|
||||||
expect(parsed.mcpServers.github_ci).toBeDefined();
|
|
||||||
expect(consoleWarningSpy).toHaveBeenCalledWith(
|
|
||||||
expect.stringContaining(
|
|
||||||
"The github_ci MCP server requires 'actions: read' permission",
|
|
||||||
),
|
|
||||||
);
|
|
||||||
|
|
||||||
process.env.DEFAULT_WORKFLOW_TOKEN = oldTokenEnv;
|
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
Reference in New Issue
Block a user