diff --git a/src/create-prompt/index.ts b/src/create-prompt/index.ts index e292f34..bdddd1b 100644 --- a/src/create-prompt/index.ts +++ b/src/create-prompt/index.ts @@ -35,38 +35,35 @@ const BASE_ALLOWED_TOOLS = [ ]; const DISALLOWED_TOOLS = ["WebSearch", "WebFetch"]; -export function buildAllowedToolsString(customAllowedTools?: string): string { +export function buildAllowedToolsString(customAllowedTools?: string[]): string { let baseTools = [...BASE_ALLOWED_TOOLS]; let allAllowedTools = baseTools.join(","); - if (customAllowedTools) { - allAllowedTools = `${allAllowedTools},${customAllowedTools}`; + if (customAllowedTools && customAllowedTools.length > 0) { + allAllowedTools = `${allAllowedTools},${customAllowedTools.join(",")}`; } return allAllowedTools; } export function buildDisallowedToolsString( - customDisallowedTools?: string, - allowedTools?: string, + customDisallowedTools?: string[], + allowedTools?: string[], ): string { let disallowedTools = [...DISALLOWED_TOOLS]; // If user has explicitly allowed some hardcoded disallowed tools, remove them from disallowed list - if (allowedTools) { - const allowedToolsArray = allowedTools - .split(",") - .map((tool) => tool.trim()); + if (allowedTools && allowedTools.length > 0) { disallowedTools = disallowedTools.filter( - (tool) => !allowedToolsArray.includes(tool), + (tool) => !allowedTools.includes(tool), ); } let allDisallowedTools = disallowedTools.join(","); - if (customDisallowedTools) { + if (customDisallowedTools && customDisallowedTools.length > 0) { if (allDisallowedTools) { - allDisallowedTools = `${allDisallowedTools},${customDisallowedTools}`; + allDisallowedTools = `${allDisallowedTools},${customDisallowedTools.join(",")}`; } else { - allDisallowedTools = customDisallowedTools; + allDisallowedTools = customDisallowedTools.join(","); } } return allDisallowedTools; @@ -120,8 +117,8 @@ export function prepareContext( triggerPhrase, ...(triggerUsername && { triggerUsername }), ...(customInstructions && { customInstructions }), - ...(allowedTools && { allowedTools }), - ...(disallowedTools && { disallowedTools }), + ...(allowedTools.length > 0 && { allowedTools: allowedTools.join(",") }), + ...(disallowedTools.length > 0 && { disallowedTools: disallowedTools.join(",") }), ...(directPrompt && { directPrompt }), ...(claudeBranch && { claudeBranch }), }; @@ -636,11 +633,11 @@ export async function createPrompt( // Set allowed tools const allAllowedTools = buildAllowedToolsString( - preparedContext.allowedTools, + context.inputs.allowedTools, ); const allDisallowedTools = buildDisallowedToolsString( - preparedContext.disallowedTools, - preparedContext.allowedTools, + context.inputs.disallowedTools, + context.inputs.allowedTools, ); core.exportVariable("ALLOWED_TOOLS", allAllowedTools); diff --git a/src/entrypoints/prepare.ts b/src/entrypoints/prepare.ts index 939f5bf..6b240d8 100644 --- a/src/entrypoints/prepare.ts +++ b/src/entrypoints/prepare.ts @@ -85,7 +85,6 @@ async function run() { // Step 11: Get MCP configuration const additionalMcpConfig = process.env.MCP_CONFIG || ""; - const allowedTools = process.env.ALLOWED_TOOLS || ""; const mcpConfig = await prepareMcpConfig({ githubToken, owner: context.repository.owner, @@ -93,7 +92,7 @@ async function run() { branch: branchInfo.currentBranch, additionalMcpConfig, claudeCommentId: commentId.toString(), - allowedTools, + allowedTools: context.inputs.allowedTools, }); core.setOutput("mcp_config", mcpConfig); } catch (error) { diff --git a/src/github/context.ts b/src/github/context.ts index 0fb7f65..1e19303 100644 --- a/src/github/context.ts +++ b/src/github/context.ts @@ -28,8 +28,8 @@ export type ParsedGitHubContext = { inputs: { triggerPhrase: string; assigneeTrigger: string; - allowedTools: string; - disallowedTools: string; + allowedTools: string[]; + disallowedTools: string[]; customInstructions: string; directPrompt: string; baseBranch?: string; @@ -52,8 +52,14 @@ export function parseGitHubContext(): ParsedGitHubContext { inputs: { triggerPhrase: process.env.TRIGGER_PHRASE ?? "@claude", assigneeTrigger: process.env.ASSIGNEE_TRIGGER ?? "", - allowedTools: process.env.ALLOWED_TOOLS ?? "", - disallowedTools: process.env.DISALLOWED_TOOLS ?? "", + allowedTools: (process.env.ALLOWED_TOOLS ?? "") + .split(",") + .map((tool) => tool.trim()) + .filter((tool) => tool.length > 0), + disallowedTools: (process.env.DISALLOWED_TOOLS ?? "") + .split(",") + .map((tool) => tool.trim()) + .filter((tool) => tool.length > 0), customInstructions: process.env.CUSTOM_INSTRUCTIONS ?? "", directPrompt: process.env.DIRECT_PROMPT ?? "", baseBranch: process.env.BASE_BRANCH, diff --git a/src/mcp/install-mcp-server.ts b/src/mcp/install-mcp-server.ts index ff20269..0eba6af 100644 --- a/src/mcp/install-mcp-server.ts +++ b/src/mcp/install-mcp-server.ts @@ -7,7 +7,7 @@ type PrepareConfigParams = { branch: string; additionalMcpConfig?: string; claudeCommentId?: string; - allowedTools?: string; + allowedTools: string[]; }; export async function prepareMcpConfig( @@ -23,22 +23,36 @@ export async function prepareMcpConfig( allowedTools, } = params; try { - // Parse allowed tools into an array - const allowedToolsList = allowedTools - ? allowedTools.split(",").map((tool) => tool.trim()) - : []; + const allowedToolsList = allowedTools || []; - // Check if any GitHub MCP tools are allowed const hasGitHubMcpTools = allowedToolsList.some((tool) => tool.startsWith("mcp__github__"), ); - // Start with an empty servers object - const mcpServers: Record = {}; + const baseMcpConfig: { mcpServers: Record } = { + mcpServers: { + github_file_ops: { + command: "bun", + args: [ + "run", + `${process.env.GITHUB_ACTION_PATH}/src/mcp/github-file-ops-server.ts`, + ], + env: { + GITHUB_TOKEN: githubToken, + REPO_OWNER: owner, + REPO_NAME: repo, + BRANCH_NAME: branch, + REPO_DIR: process.env.GITHUB_WORKSPACE || process.cwd(), + ...(claudeCommentId && { CLAUDE_COMMENT_ID: claudeCommentId }), + GITHUB_EVENT_NAME: process.env.GITHUB_EVENT_NAME || "", + IS_PR: process.env.IS_PR || "false", + }, + }, + }, + }; - // Only include github MCP server if GitHub tools are allowed if (hasGitHubMcpTools) { - mcpServers.github = { + baseMcpConfig.mcpServers.github = { command: "docker", args: [ "run", @@ -54,31 +68,6 @@ export async function prepareMcpConfig( }; } - // Always include github_file_ops server as it contains essential tools - // (mcp__github_file_ops__commit_files, mcp__github_file_ops__update_claude_comment) - // These are in the BASE_ALLOWED_TOOLS and should always be available - mcpServers.github_file_ops = { - command: "bun", - args: [ - "run", - `${process.env.GITHUB_ACTION_PATH}/src/mcp/github-file-ops-server.ts`, - ], - env: { - GITHUB_TOKEN: githubToken, - REPO_OWNER: owner, - REPO_NAME: repo, - BRANCH_NAME: branch, - REPO_DIR: process.env.GITHUB_WORKSPACE || process.cwd(), - ...(claudeCommentId && { CLAUDE_COMMENT_ID: claudeCommentId }), - GITHUB_EVENT_NAME: process.env.GITHUB_EVENT_NAME || "", - IS_PR: process.env.IS_PR || "false", - }, - }; - - const baseMcpConfig = { - mcpServers, - }; - // Merge with additional MCP config if provided if (additionalMcpConfig && additionalMcpConfig.trim()) { try { diff --git a/test/create-prompt.test.ts b/test/create-prompt.test.ts index 617f0ac..65c5625 100644 --- a/test/create-prompt.test.ts +++ b/test/create-prompt.test.ts @@ -652,7 +652,7 @@ describe("buildAllowedToolsString", () => { }); test("should append custom tools when provided", () => { - const customTools = "Tool1,Tool2,Tool3"; + const customTools = ["Tool1", "Tool2", "Tool3"]; const result = buildAllowedToolsString(customTools); // Base tools should be present @@ -683,7 +683,7 @@ describe("buildDisallowedToolsString", () => { }); test("should append custom disallowed tools when provided", () => { - const customDisallowedTools = "BadTool1,BadTool2"; + const customDisallowedTools = ["BadTool1", "BadTool2"]; const result = buildDisallowedToolsString(customDisallowedTools); // Base disallowed tools should be present @@ -701,8 +701,8 @@ describe("buildDisallowedToolsString", () => { }); test("should remove hardcoded disallowed tools if they are in allowed tools", () => { - const customDisallowedTools = "BadTool1,BadTool2"; - const allowedTools = "WebSearch,SomeOtherTool"; + const customDisallowedTools = ["BadTool1", "BadTool2"]; + const allowedTools = ["WebSearch", "SomeOtherTool"]; const result = buildDisallowedToolsString( customDisallowedTools, allowedTools, @@ -720,7 +720,7 @@ describe("buildDisallowedToolsString", () => { }); test("should remove all hardcoded disallowed tools if they are all in allowed tools", () => { - const allowedTools = "WebSearch,WebFetch,SomeOtherTool"; + const allowedTools = ["WebSearch", "WebFetch", "SomeOtherTool"]; const result = buildDisallowedToolsString(undefined, allowedTools); // Both hardcoded disallowed tools should be removed @@ -732,8 +732,8 @@ describe("buildDisallowedToolsString", () => { }); test("should handle custom disallowed tools when all hardcoded tools are overridden", () => { - const customDisallowedTools = "BadTool1,BadTool2"; - const allowedTools = "WebSearch,WebFetch"; + const customDisallowedTools = ["BadTool1", "BadTool2"]; + const allowedTools = ["WebSearch", "WebFetch"]; const result = buildDisallowedToolsString( customDisallowedTools, allowedTools, diff --git a/test/install-mcp-server.test.ts b/test/install-mcp-server.test.ts index 5f6d1b3..b02b7db 100644 --- a/test/install-mcp-server.test.ts +++ b/test/install-mcp-server.test.ts @@ -52,8 +52,10 @@ describe("prepareMcpConfig", () => { owner: "test-owner", repo: "test-repo", branch: "test-branch", - allowedTools: - "mcp__github__create_issue,mcp__github_file_ops__commit_files", + allowedTools: [ + "mcp__github__create_issue", + "mcp__github_file_ops__commit_files", + ], }); const parsed = JSON.parse(result); @@ -71,8 +73,10 @@ describe("prepareMcpConfig", () => { owner: "test-owner", repo: "test-repo", branch: "test-branch", - allowedTools: - "mcp__github_file_ops__commit_files,mcp__github_file_ops__update_claude_comment", + allowedTools: [ + "mcp__github_file_ops__commit_files", + "mcp__github_file_ops__update_claude_comment", + ], }); const parsed = JSON.parse(result); @@ -87,7 +91,7 @@ describe("prepareMcpConfig", () => { owner: "test-owner", repo: "test-repo", branch: "test-branch", - allowedTools: "Edit,Read,Write", + allowedTools: ["Edit", "Read", "Write"], }); const parsed = JSON.parse(result); @@ -147,8 +151,10 @@ describe("prepareMcpConfig", () => { repo: "test-repo", branch: "test-branch", additionalMcpConfig: additionalConfig, - allowedTools: - "mcp__github__create_issue,mcp__github_file_ops__commit_files", + allowedTools: [ + "mcp__github__create_issue", + "mcp__github_file_ops__commit_files", + ], }); const parsed = JSON.parse(result); @@ -182,8 +188,10 @@ describe("prepareMcpConfig", () => { repo: "test-repo", branch: "test-branch", additionalMcpConfig: additionalConfig, - allowedTools: - "mcp__github__create_issue,mcp__github_file_ops__commit_files", + allowedTools: [ + "mcp__github__create_issue", + "mcp__github_file_ops__commit_files", + ], }); const parsed = JSON.parse(result); diff --git a/test/mockContext.ts b/test/mockContext.ts index c93acc1..692137c 100644 --- a/test/mockContext.ts +++ b/test/mockContext.ts @@ -11,8 +11,8 @@ const defaultInputs = { triggerPhrase: "/claude", assigneeTrigger: "", anthropicModel: "claude-3-7-sonnet-20250219", - allowedTools: "", - disallowedTools: "", + allowedTools: [] as string[], + disallowedTools: [] as string[], customInstructions: "", directPrompt: "", useBedrock: false, diff --git a/test/permissions.test.ts b/test/permissions.test.ts index 931d873..61e2ca9 100644 --- a/test/permissions.test.ts +++ b/test/permissions.test.ts @@ -62,8 +62,8 @@ describe("checkWritePermissions", () => { inputs: { triggerPhrase: "@claude", assigneeTrigger: "", - allowedTools: "", - disallowedTools: "", + allowedTools: [], + disallowedTools: [], customInstructions: "", directPrompt: "", }, diff --git a/test/prepare-context.test.ts b/test/prepare-context.test.ts index 5be89f0..7811c5b 100644 --- a/test/prepare-context.test.ts +++ b/test/prepare-context.test.ts @@ -242,7 +242,7 @@ describe("parseEnvVarsWithContext", () => { ...mockPullRequestCommentContext, inputs: { ...mockPullRequestCommentContext.inputs, - allowedTools: "Tool1,Tool2", + allowedTools: ["Tool1", "Tool2"], }, }); const result = prepareContext(contextWithAllowedTools, "12345"); diff --git a/test/trigger-validation.test.ts b/test/trigger-validation.test.ts index ae5d6d3..bbe40bd 100644 --- a/test/trigger-validation.test.ts +++ b/test/trigger-validation.test.ts @@ -30,8 +30,8 @@ describe("checkContainsTrigger", () => { triggerPhrase: "/claude", assigneeTrigger: "", directPrompt: "Fix the bug in the login form", - allowedTools: "", - disallowedTools: "", + allowedTools: [], + disallowedTools: [], customInstructions: "", }, }); @@ -56,8 +56,8 @@ describe("checkContainsTrigger", () => { triggerPhrase: "/claude", assigneeTrigger: "", directPrompt: "", - allowedTools: "", - disallowedTools: "", + allowedTools: [], + disallowedTools: [], customInstructions: "", }, }); @@ -228,8 +228,8 @@ describe("checkContainsTrigger", () => { triggerPhrase: "@claude", assigneeTrigger: "", directPrompt: "", - allowedTools: "", - disallowedTools: "", + allowedTools: [], + disallowedTools: [], customInstructions: "", }, }); @@ -255,8 +255,8 @@ describe("checkContainsTrigger", () => { triggerPhrase: "@claude", assigneeTrigger: "", directPrompt: "", - allowedTools: "", - disallowedTools: "", + allowedTools: [], + disallowedTools: [], customInstructions: "", }, }); @@ -282,8 +282,8 @@ describe("checkContainsTrigger", () => { triggerPhrase: "@claude", assigneeTrigger: "", directPrompt: "", - allowedTools: "", - disallowedTools: "", + allowedTools: [], + disallowedTools: [], customInstructions: "", }, });