diff --git a/action.yml b/action.yml index 18b26cd..84132ce 100644 --- a/action.yml +++ b/action.yml @@ -92,6 +92,10 @@ inputs: description: "Use just one comment to deliver issue/PR comments" required: false default: "false" + use_commit_signing: + description: "Enable commit signing using GitHub's commit signature verification. When false, Claude uses standard git commands" + required: false + default: "false" outputs: execution_file: @@ -133,6 +137,7 @@ runs: USE_STICKY_COMMENT: ${{ inputs.use_sticky_comment }} ACTIONS_TOKEN: ${{ github.token }} ADDITIONAL_PERMISSIONS: ${{ inputs.additional_permissions }} + USE_COMMIT_SIGNING: ${{ inputs.use_commit_signing }} - name: Run Claude Code id: claude-code @@ -201,6 +206,7 @@ runs: PREPARE_SUCCESS: ${{ steps.prepare.outcome == 'success' }} PREPARE_ERROR: ${{ steps.prepare.outputs.prepare_error || '' }} USE_STICKY_COMMENT: ${{ inputs.use_sticky_comment }} + USE_COMMIT_SIGNING: ${{ inputs.use_commit_signing }} - name: Display Claude Code Report if: steps.prepare.outputs.contains_trigger == 'true' && steps.claude-code.outputs.execution_file != '' diff --git a/src/create-prompt/index.ts b/src/create-prompt/index.ts index ad91179..0985f70 100644 --- a/src/create-prompt/index.ts +++ b/src/create-prompt/index.ts @@ -30,18 +30,40 @@ const BASE_ALLOWED_TOOLS = [ "LS", "Read", "Write", - "mcp__github_file_ops__commit_files", - "mcp__github_file_ops__delete_files", - "mcp__github_file_ops__update_claude_comment", ]; const DISALLOWED_TOOLS = ["WebSearch", "WebFetch"]; export function buildAllowedToolsString( customAllowedTools?: string[], includeActionsTools: boolean = false, + useCommitSigning: boolean = false, ): string { let baseTools = [...BASE_ALLOWED_TOOLS]; + // Always include the comment update tool from the comment server + baseTools.push("mcp__github_comment__update_claude_comment"); + + // Add commit signing tools if enabled + if (useCommitSigning) { + baseTools.push( + "mcp__github_file_ops__commit_files", + "mcp__github_file_ops__delete_files", + ); + } else { + // When not using commit signing, add specific Bash git commands only + baseTools.push( + "Bash(git add:*)", + "Bash(git commit:*)", + "Bash(git push:*)", + "Bash(git status:*)", + "Bash(git diff:*)", + "Bash(git log:*)", + "Bash(git rm:*)", + "Bash(git config user.name:*)", + "Bash(git config user.email:*)", + ); + } + // Add GitHub Actions MCP tools if enabled if (includeActionsTools) { baseTools.push( @@ -380,9 +402,68 @@ export function getEventTypeAndContext(envVars: PreparedContext): { } } +function getCommitInstructions( + eventData: EventData, + githubData: FetchDataResult, + context: PreparedContext, + useCommitSigning: boolean, +): string { + const coAuthorLine = + (githubData.triggerDisplayName ?? context.triggerUsername !== "Unknown") + ? `Co-authored-by: ${githubData.triggerDisplayName ?? context.triggerUsername} <${context.triggerUsername}@users.noreply.github.com>` + : ""; + + if (useCommitSigning) { + if (eventData.isPR && !eventData.claudeBranch) { + return ` + - Push directly using mcp__github_file_ops__commit_files to the existing branch (works for both new and existing files). + - Use mcp__github_file_ops__commit_files to commit files atomically in a single commit (supports single or multiple files). + - When pushing changes with this tool and the trigger user is not "Unknown", include a Co-authored-by trailer in the commit message. + - Use: "${coAuthorLine}"`; + } else { + return ` + - You are already on the correct branch (${eventData.claudeBranch || "the PR branch"}). Do not create a new branch. + - Push changes directly to the current branch using mcp__github_file_ops__commit_files (works for both new and existing files) + - Use mcp__github_file_ops__commit_files to commit files atomically in a single commit (supports single or multiple files). + - When pushing changes and the trigger user is not "Unknown", include a Co-authored-by trailer in the commit message. + - Use: "${coAuthorLine}"`; + } + } else { + // Non-signing instructions + if (eventData.isPR && !eventData.claudeBranch) { + return ` + - Use git commands via the Bash tool to commit and push your changes: + - Stage files: Bash(git add ) + - Commit with a descriptive message: Bash(git commit -m "") + ${ + coAuthorLine + ? `- When committing and the trigger user is not "Unknown", include a Co-authored-by trailer: + Bash(git commit -m "\\n\\n${coAuthorLine}")` + : "" + } + - Push to the remote: Bash(git push origin HEAD)`; + } else { + const branchName = eventData.claudeBranch || eventData.baseBranch; + return ` + - You are already on the correct branch (${eventData.claudeBranch || "the PR branch"}). Do not create a new branch. + - Use git commands via the Bash tool to commit and push your changes: + - Stage files: Bash(git add ) + - Commit with a descriptive message: Bash(git commit -m "") + ${ + coAuthorLine + ? `- When committing and the trigger user is not "Unknown", include a Co-authored-by trailer: + Bash(git commit -m "\\n\\n${coAuthorLine}")` + : "" + } + - Push to the remote: Bash(git push origin ${branchName})`; + } + } +} + export function generatePrompt( context: PreparedContext, githubData: FetchDataResult, + useCommitSigning: boolean, ): string { const { contextData, @@ -471,9 +552,9 @@ ${sanitizeContent(context.directPrompt)} : "" } ${` -IMPORTANT: You have been provided with the mcp__github_file_ops__update_claude_comment tool to update your comment. This tool automatically handles both issue and PR comments. +IMPORTANT: You have been provided with the mcp__github_comment__update_claude_comment tool to update your comment. This tool automatically handles both issue and PR comments. -Tool usage example for mcp__github_file_ops__update_claude_comment: +Tool usage example for mcp__github_comment__update_claude_comment: { "body": "Your comment text here" } @@ -492,7 +573,7 @@ Follow these steps: 1. Create a Todo List: - Use your GitHub comment to maintain a detailed task list based on the request. - Format todos as a checklist (- [ ] for incomplete, - [x] for complete). - - Update the comment using mcp__github_file_ops__update_claude_comment with each task completion. + - Update the comment using mcp__github_comment__update_claude_comment with each task completion. 2. Gather Context: - Analyze the pre-fetched data provided above. @@ -523,29 +604,16 @@ ${context.directPrompt ? ` - DIRECT INSTRUCTION: A direct instruction was prov - Look for bugs, security issues, performance problems, and other issues - Suggest improvements for readability and maintainability - Check for best practices and coding standards - - Reference specific code sections with file paths and line numbers${eventData.isPR ? "\n - AFTER reading files and analyzing code, you MUST call mcp__github_file_ops__update_claude_comment to post your review" : ""} + - Reference specific code sections with file paths and line numbers${eventData.isPR ? `\n - AFTER reading files and analyzing code, you MUST call mcp__github_comment__update_claude_comment to post your review` : ""} - Formulate a concise, technical, and helpful response based on the context. - Reference specific code with inline formatting or code blocks. - Include relevant file paths and line numbers when applicable. - - ${eventData.isPR ? "IMPORTANT: Submit your review feedback by updating the Claude comment using mcp__github_file_ops__update_claude_comment. This will be displayed as your PR review." : "Remember that this feedback must be posted to the GitHub comment using mcp__github_file_ops__update_claude_comment."} + - ${eventData.isPR ? `IMPORTANT: Submit your review feedback by updating the Claude comment using mcp__github_comment__update_claude_comment. This will be displayed as your PR review.` : `Remember that this feedback must be posted to the GitHub comment using mcp__github_comment__update_claude_comment.`} B. For Straightforward Changes: - Use file system tools to make the change locally. - If you discover related tasks (e.g., updating tests), add them to the todo list. - - Mark each subtask as completed as you progress. - ${ - eventData.isPR && !eventData.claudeBranch - ? ` - - Push directly using mcp__github_file_ops__commit_files to the existing branch (works for both new and existing files). - - Use mcp__github_file_ops__commit_files to commit files atomically in a single commit (supports single or multiple files). - - When pushing changes with this tool and the trigger user is not "Unknown", include a Co-authored-by trailer in the commit message. - - Use: "Co-authored-by: ${githubData.triggerDisplayName ?? context.triggerUsername} <${context.triggerUsername}@users.noreply.github.com>"` - : ` - - You are already on the correct branch (${eventData.claudeBranch || "the PR branch"}). Do not create a new branch. - - Push changes directly to the current branch using mcp__github_file_ops__commit_files (works for both new and existing files) - - Use mcp__github_file_ops__commit_files to commit files atomically in a single commit (supports single or multiple files). - - When pushing changes and the trigger user is not "Unknown", include a Co-authored-by trailer in the commit message. - - Use: "Co-authored-by: ${githubData.triggerDisplayName ?? context.triggerUsername} <${context.triggerUsername}@users.noreply.github.com>" + - Mark each subtask as completed as you progress.${getCommitInstructions(eventData, githubData, context, useCommitSigning)} ${ eventData.claudeBranch ? `- Provide a URL to create a PR manually in this format: @@ -563,7 +631,6 @@ ${context.directPrompt ? ` - DIRECT INSTRUCTION: A direct instruction was prov - The signature: "Generated with [Claude Code](https://claude.ai/code)" - Just include the markdown link with text "Create a PR" - do not add explanatory text before it like "You can create a PR using this link"` : "" - }` } C. For Complex Changes: @@ -579,20 +646,31 @@ ${context.directPrompt ? ` - DIRECT INSTRUCTION: A direct instruction was prov - Always update the GitHub comment to reflect the current todo state. - When all todos are completed, remove the spinner and add a brief summary of what was accomplished, and what was not done. - Note: If you see previous Claude comments with headers like "**Claude finished @user's task**" followed by "---", do not include this in your comment. The system adds this automatically. - - If you changed any files locally, you must update them in the remote branch via mcp__github_file_ops__commit_files before saying that you're done. + - If you changed any files locally, you must update them in the remote branch via ${useCommitSigning ? "mcp__github_file_ops__commit_files" : "git commands (add, commit, push)"} before saying that you're done. ${eventData.claudeBranch ? `- If you created anything in your branch, your comment must include the PR URL with prefilled title and body mentioned above.` : ""} Important Notes: - All communication must happen through GitHub PR comments. -- Never create new comments. Only update the existing comment using mcp__github_file_ops__update_claude_comment. -- This includes ALL responses: code reviews, answers to questions, progress updates, and final results.${eventData.isPR ? "\n- PR CRITICAL: After reading files and forming your response, you MUST post it by calling mcp__github_file_ops__update_claude_comment. Do NOT just respond with a normal response, the user will not see it." : ""} +- Never create new comments. Only update the existing comment using mcp__github_comment__update_claude_comment. +- This includes ALL responses: code reviews, answers to questions, progress updates, and final results.${eventData.isPR ? `\n- PR CRITICAL: After reading files and forming your response, you MUST post it by calling mcp__github_comment__update_claude_comment. Do NOT just respond with a normal response, the user will not see it.` : ""} - You communicate exclusively by editing your single comment - not through any other means. - Use this spinner HTML when work is in progress: ${eventData.isPR && !eventData.claudeBranch ? `- Always push to the existing branch when triggered on a PR.` : `- IMPORTANT: You are already on the correct branch (${eventData.claudeBranch || "the created branch"}). Never create new branches when triggered on issues or closed/merged PRs.`} -- Use mcp__github_file_ops__commit_files for making commits (works for both new and existing files, single or multiple). Use mcp__github_file_ops__delete_files for deleting files (supports deleting single or multiple files atomically), or mcp__github__delete_file for deleting a single file. Edit files locally, and the tool will read the content from the same path on disk. +${ + useCommitSigning + ? `- Use mcp__github_file_ops__commit_files for making commits (works for both new and existing files, single or multiple). Use mcp__github_file_ops__delete_files for deleting files (supports deleting single or multiple files atomically), or mcp__github__delete_file for deleting a single file. Edit files locally, and the tool will read the content from the same path on disk. Tool usage examples: - mcp__github_file_ops__commit_files: {"files": ["path/to/file1.js", "path/to/file2.py"], "message": "feat: add new feature"} - - mcp__github_file_ops__delete_files: {"files": ["path/to/old.js"], "message": "chore: remove deprecated file"} + - mcp__github_file_ops__delete_files: {"files": ["path/to/old.js"], "message": "chore: remove deprecated file"}` + : `- Use git commands via the Bash tool for version control (you have access to specific git commands only): + - Stage files: Bash(git add ) + - Commit changes: Bash(git commit -m "") + - Push to remote: Bash(git push origin ) (NEVER force push) + - Delete files: Bash(git rm ) followed by commit and push + - Check status: Bash(git status) + - View diff: Bash(git diff) + - Configure git user: Bash(git config user.name "...") and Bash(git config user.email "...")` +} - Display the todo list as a checklist in the GitHub comment and mark things off as you go. - REPOSITORY SETUP INSTRUCTIONS: The repository's CLAUDE.md file(s) contain critical repo-specific setup instructions, development guidelines, and preferences. Always read and follow these files, particularly the root CLAUDE.md, as they provide essential context for working with the codebase effectively. - Use h3 headers (###) for section titles in your comments, not h1 headers (#). @@ -663,7 +741,11 @@ export async function createPrompt( }); // Generate the prompt - const promptContent = generatePrompt(preparedContext, githubData); + const promptContent = generatePrompt( + preparedContext, + githubData, + context.inputs.useCommitSigning, + ); // Log the final prompt to console console.log("===== FINAL PROMPT ====="); @@ -683,6 +765,7 @@ export async function createPrompt( const allAllowedTools = buildAllowedToolsString( context.inputs.allowedTools, hasActionsReadPermission, + context.inputs.useCommitSigning, ); const allDisallowedTools = buildDisallowedToolsString( context.inputs.disallowedTools, diff --git a/src/entrypoints/prepare.ts b/src/entrypoints/prepare.ts index 23bb74b..257d7f8 100644 --- a/src/entrypoints/prepare.ts +++ b/src/entrypoints/prepare.ts @@ -13,6 +13,7 @@ import { checkWritePermissions } from "../github/validation/permissions"; import { createInitialComment } from "../github/operations/comments/create-initial"; import { setupBranch } from "../github/operations/branch"; import { updateTrackingComment } from "../github/operations/comments/update-with-branch"; +import { configureGitAuth } from "../github/operations/git-config"; import { prepareMcpConfig } from "../mcp/install-mcp-server"; import { createPrompt } from "../create-prompt"; import { createOctokit } from "../github/api/client"; @@ -51,7 +52,8 @@ async function run() { await checkHumanActor(octokit.rest, context); // Step 6: Create initial tracking comment - const commentId = await createInitialComment(octokit.rest, context); + const commentData = await createInitialComment(octokit.rest, context); + const commentId = commentData.id; // Step 7: Fetch GitHub data (once for both branch setup and prompt creation) const githubData = await fetchGitHubData({ @@ -75,7 +77,17 @@ async function run() { ); } - // Step 10: Create prompt file + // Step 10: Configure git authentication if not using commit signing + if (!context.inputs.useCommitSigning) { + try { + await configureGitAuth(githubToken, context, commentData.user); + } catch (error) { + console.error("Failed to configure git authentication:", error); + throw error; + } + } + + // Step 11: Create prompt file await createPrompt( commentId, branchInfo.baseBranch, @@ -84,7 +96,7 @@ async function run() { context, ); - // Step 11: Get MCP configuration + // Step 12: Get MCP configuration const additionalMcpConfig = process.env.MCP_CONFIG || ""; const mcpConfig = await prepareMcpConfig({ githubToken, diff --git a/src/entrypoints/update-comment-link.ts b/src/entrypoints/update-comment-link.ts index 9090373..4664691 100644 --- a/src/entrypoints/update-comment-link.ts +++ b/src/entrypoints/update-comment-link.ts @@ -11,7 +11,7 @@ import { isPullRequestReviewCommentEvent, } from "../github/context"; import { GITHUB_SERVER_URL } from "../github/api/config"; -import { checkAndDeleteEmptyBranch } from "../github/operations/branch-cleanup"; +import { checkAndCommitOrDeleteBranch } from "../github/operations/branch-cleanup"; import { updateClaudeComment } from "../github/operations/comments/update-claude-comment"; async function run() { @@ -88,13 +88,16 @@ async function run() { const currentBody = comment.body ?? ""; // Check if we need to add branch link for new branches - const { shouldDeleteBranch, branchLink } = await checkAndDeleteEmptyBranch( - octokit, - owner, - repo, - claudeBranch, - baseBranch, - ); + const useCommitSigning = process.env.USE_COMMIT_SIGNING === "true"; + const { shouldDeleteBranch, branchLink } = + await checkAndCommitOrDeleteBranch( + octokit, + owner, + repo, + claudeBranch, + baseBranch, + useCommitSigning, + ); // Check if we need to add PR URL when we have a new branch let prLink = ""; diff --git a/src/github/context.ts b/src/github/context.ts index 205a955..c156b54 100644 --- a/src/github/context.ts +++ b/src/github/context.ts @@ -38,6 +38,7 @@ export type ParsedGitHubContext = { branchPrefix: string; useStickyComment: boolean; additionalPermissions: Map; + useCommitSigning: boolean; }; }; @@ -68,6 +69,7 @@ export function parseGitHubContext(): ParsedGitHubContext { additionalPermissions: parseAdditionalPermissions( process.env.ADDITIONAL_PERMISSIONS ?? "", ), + useCommitSigning: process.env.USE_COMMIT_SIGNING === "true", }, }; diff --git a/src/github/operations/branch-cleanup.ts b/src/github/operations/branch-cleanup.ts index 662a474..9ac2cef 100644 --- a/src/github/operations/branch-cleanup.ts +++ b/src/github/operations/branch-cleanup.ts @@ -1,12 +1,14 @@ import type { Octokits } from "../api/client"; import { GITHUB_SERVER_URL } from "../api/config"; +import { $ } from "bun"; -export async function checkAndDeleteEmptyBranch( +export async function checkAndCommitOrDeleteBranch( octokit: Octokits, owner: string, repo: string, claudeBranch: string | undefined, baseBranch: string, + useCommitSigning: boolean, ): Promise<{ shouldDeleteBranch: boolean; branchLink: string }> { let branchLink = ""; let shouldDeleteBranch = false; @@ -21,12 +23,58 @@ export async function checkAndDeleteEmptyBranch( basehead: `${baseBranch}...${claudeBranch}`, }); - // If there are no commits, mark branch for deletion + // If there are no commits, check for uncommitted changes if not using commit signing if (comparison.total_commits === 0) { - console.log( - `Branch ${claudeBranch} has no commits from Claude, will delete it`, - ); - shouldDeleteBranch = true; + if (!useCommitSigning) { + console.log( + `Branch ${claudeBranch} has no commits from Claude, checking for uncommitted changes...`, + ); + + // Check for uncommitted changes using git status + try { + const gitStatus = await $`git status --porcelain`.quiet(); + const hasUncommittedChanges = + gitStatus.stdout.toString().trim().length > 0; + + if (hasUncommittedChanges) { + console.log("Found uncommitted changes, committing them..."); + + // Add all changes + await $`git add -A`; + + // Commit with a descriptive message + const runId = process.env.GITHUB_RUN_ID || "unknown"; + const commitMessage = `Auto-commit: Save uncommitted changes from Claude\n\nRun ID: ${runId}`; + await $`git commit -m ${commitMessage}`; + + // Push the changes + await $`git push origin ${claudeBranch}`; + + console.log( + "✅ Successfully committed and pushed uncommitted changes", + ); + + // Set branch link since we now have commits + const branchUrl = `${GITHUB_SERVER_URL}/${owner}/${repo}/tree/${claudeBranch}`; + branchLink = `\n[View branch](${branchUrl})`; + } else { + console.log( + "No uncommitted changes found, marking branch for deletion", + ); + shouldDeleteBranch = true; + } + } catch (gitError) { + console.error("Error checking/committing changes:", gitError); + // If we can't check git status, assume the branch might have changes + const branchUrl = `${GITHUB_SERVER_URL}/${owner}/${repo}/tree/${claudeBranch}`; + branchLink = `\n[View branch](${branchUrl})`; + } + } else { + console.log( + `Branch ${claudeBranch} has no commits from Claude, will delete it`, + ); + shouldDeleteBranch = true; + } } else { // Only add branch link if there are commits const branchUrl = `${GITHUB_SERVER_URL}/${owner}/${repo}/tree/${claudeBranch}`; diff --git a/src/github/operations/comments/create-initial.ts b/src/github/operations/comments/create-initial.ts index 2bac476..1243035 100644 --- a/src/github/operations/comments/create-initial.ts +++ b/src/github/operations/comments/create-initial.ts @@ -86,7 +86,7 @@ export async function createInitialComment( const githubOutput = process.env.GITHUB_OUTPUT!; appendFileSync(githubOutput, `claude_comment_id=${response.data.id}\n`); console.log(`✅ Created initial comment with ID: ${response.data.id}`); - return response.data.id; + return response.data; } catch (error) { console.error("Error in initial comment:", error); @@ -102,7 +102,7 @@ export async function createInitialComment( const githubOutput = process.env.GITHUB_OUTPUT!; appendFileSync(githubOutput, `claude_comment_id=${response.data.id}\n`); console.log(`✅ Created fallback comment with ID: ${response.data.id}`); - return response.data.id; + return response.data; } catch (fallbackError) { console.error("Error creating fallback comment:", fallbackError); throw fallbackError; diff --git a/src/github/operations/git-config.ts b/src/github/operations/git-config.ts new file mode 100644 index 0000000..bc9969f --- /dev/null +++ b/src/github/operations/git-config.ts @@ -0,0 +1,56 @@ +#!/usr/bin/env bun + +/** + * Configure git authentication for non-signing mode + * Sets up git user and authentication to work with GitHub App tokens + */ + +import { $ } from "bun"; +import type { ParsedGitHubContext } from "../context"; +import { GITHUB_SERVER_URL } from "../api/config"; + +type GitUser = { + login: string; + id: number; +}; + +export async function configureGitAuth( + githubToken: string, + context: ParsedGitHubContext, + user: GitUser | null, +) { + console.log("Configuring git authentication for non-signing mode"); + + // Configure git user based on the comment creator + console.log("Configuring git user..."); + if (user) { + const botName = user.login; + const botId = user.id; + console.log(`Setting git user as ${botName}...`); + await $`git config user.name "${botName}"`; + await $`git config user.email "${botId}+${botName}@users.noreply.github.com"`; + console.log(`✓ Set git user as ${botName}`); + } else { + console.log("No user data in comment, using default bot user"); + await $`git config user.name "github-actions[bot]"`; + await $`git config user.email "41898282+github-actions[bot]@users.noreply.github.com"`; + } + + // Remove the authorization header that actions/checkout sets + console.log("Removing existing git authentication headers..."); + try { + await $`git config --unset-all http.${GITHUB_SERVER_URL}/.extraheader`; + console.log("✓ Removed existing authentication headers"); + } catch (e) { + console.log("No existing authentication headers to remove"); + } + + // Update the remote URL to include the token for authentication + console.log("Updating remote URL with authentication..."); + const serverUrl = new URL(GITHUB_SERVER_URL); + const remoteUrl = `https://x-access-token:${githubToken}@${serverUrl.host}/${context.repository.owner}/${context.repository.repo}.git`; + await $`git remote set-url origin ${remoteUrl}`; + console.log("✓ Updated remote URL with authentication token"); + + console.log("Git authentication configured successfully"); +} diff --git a/src/mcp/github-comment-server.ts b/src/mcp/github-comment-server.ts new file mode 100644 index 0000000..18ab6a2 --- /dev/null +++ b/src/mcp/github-comment-server.ts @@ -0,0 +1,98 @@ +#!/usr/bin/env node +// GitHub Comment MCP Server - Minimal server that only provides comment update functionality +import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; +import { StdioServerTransport } from "@modelcontextprotocol/sdk/server/stdio.js"; +import { z } from "zod"; +import { GITHUB_API_URL } from "../github/api/config"; +import { Octokit } from "@octokit/rest"; +import { updateClaudeComment } from "../github/operations/comments/update-claude-comment"; + +// Get repository information from environment variables +const REPO_OWNER = process.env.REPO_OWNER; +const REPO_NAME = process.env.REPO_NAME; + +if (!REPO_OWNER || !REPO_NAME) { + console.error( + "Error: REPO_OWNER and REPO_NAME environment variables are required", + ); + process.exit(1); +} + +const server = new McpServer({ + name: "GitHub Comment Server", + version: "0.0.1", +}); + +server.tool( + "update_claude_comment", + "Update the Claude comment with progress and results (automatically handles both issue and PR comments)", + { + body: z.string().describe("The updated comment content"), + }, + async ({ body }) => { + try { + const githubToken = process.env.GITHUB_TOKEN; + const claudeCommentId = process.env.CLAUDE_COMMENT_ID; + const eventName = process.env.GITHUB_EVENT_NAME; + + if (!githubToken) { + throw new Error("GITHUB_TOKEN environment variable is required"); + } + if (!claudeCommentId) { + throw new Error("CLAUDE_COMMENT_ID environment variable is required"); + } + + const owner = REPO_OWNER; + const repo = REPO_NAME; + const commentId = parseInt(claudeCommentId, 10); + + const octokit = new Octokit({ + auth: githubToken, + baseUrl: GITHUB_API_URL, + }); + + const isPullRequestReviewComment = + eventName === "pull_request_review_comment"; + + const result = await updateClaudeComment(octokit, { + owner, + repo, + commentId, + body, + isPullRequestReviewComment, + }); + + return { + content: [ + { + type: "text", + text: JSON.stringify(result, null, 2), + }, + ], + }; + } catch (error) { + const errorMessage = + error instanceof Error ? error.message : String(error); + return { + content: [ + { + type: "text", + text: `Error: ${errorMessage}`, + }, + ], + error: errorMessage, + isError: true, + }; + } + }, +); + +async function runServer() { + const transport = new StdioServerTransport(); + await server.connect(transport); + process.on("exit", () => { + server.close(); + }); +} + +runServer().catch(console.error); diff --git a/src/mcp/github-file-ops-server.ts b/src/mcp/github-file-ops-server.ts index e00c887..4b477d2 100644 --- a/src/mcp/github-file-ops-server.ts +++ b/src/mcp/github-file-ops-server.ts @@ -7,8 +7,6 @@ import { readFile } from "fs/promises"; import { join } from "path"; import fetch from "node-fetch"; import { GITHUB_API_URL } from "../github/api/config"; -import { Octokit } from "@octokit/rest"; -import { updateClaudeComment } from "../github/operations/comments/update-claude-comment"; import { retryWithBackoff } from "../utils/retry"; type GitHubRef = { @@ -535,70 +533,6 @@ server.tool( }, ); -server.tool( - "update_claude_comment", - "Update the Claude comment with progress and results (automatically handles both issue and PR comments)", - { - body: z.string().describe("The updated comment content"), - }, - async ({ body }) => { - try { - const githubToken = process.env.GITHUB_TOKEN; - const claudeCommentId = process.env.CLAUDE_COMMENT_ID; - const eventName = process.env.GITHUB_EVENT_NAME; - - if (!githubToken) { - throw new Error("GITHUB_TOKEN environment variable is required"); - } - if (!claudeCommentId) { - throw new Error("CLAUDE_COMMENT_ID environment variable is required"); - } - - const owner = REPO_OWNER; - const repo = REPO_NAME; - const commentId = parseInt(claudeCommentId, 10); - - const octokit = new Octokit({ - auth: githubToken, - baseUrl: GITHUB_API_URL, - }); - - const isPullRequestReviewComment = - eventName === "pull_request_review_comment"; - - const result = await updateClaudeComment(octokit, { - owner, - repo, - commentId, - body, - isPullRequestReviewComment, - }); - - return { - content: [ - { - type: "text", - text: JSON.stringify(result, null, 2), - }, - ], - }; - } catch (error) { - const errorMessage = - error instanceof Error ? error.message : String(error); - return { - content: [ - { - type: "text", - text: `Error: ${errorMessage}`, - }, - ], - error: errorMessage, - isError: true, - }; - } - }, -); - async function runServer() { const transport = new StdioServerTransport(); await server.connect(transport); diff --git a/src/mcp/install-mcp-server.ts b/src/mcp/install-mcp-server.ts index 6edd6c6..8c05d00 100644 --- a/src/mcp/install-mcp-server.ts +++ b/src/mcp/install-mcp-server.ts @@ -67,28 +67,47 @@ export async function prepareMcpConfig( ); 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", - GITHUB_API_URL: GITHUB_API_URL, - }, - }, + mcpServers: {}, + }; + + // Always include comment server for updating Claude comments + baseMcpConfig.mcpServers.github_comment = { + command: "bun", + args: [ + "run", + `${process.env.GITHUB_ACTION_PATH}/src/mcp/github-comment-server.ts`, + ], + env: { + GITHUB_TOKEN: githubToken, + REPO_OWNER: owner, + REPO_NAME: repo, + ...(claudeCommentId && { CLAUDE_COMMENT_ID: claudeCommentId }), + GITHUB_EVENT_NAME: process.env.GITHUB_EVENT_NAME || "", + GITHUB_API_URL: GITHUB_API_URL, }, }; + // Include file ops server when commit signing is enabled + if (context.inputs.useCommitSigning) { + baseMcpConfig.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(), + GITHUB_EVENT_NAME: process.env.GITHUB_EVENT_NAME || "", + IS_PR: process.env.IS_PR || "false", + GITHUB_API_URL: GITHUB_API_URL, + }, + }; + } + // Only add CI server if we have actions:read permission and we're in a PR context const hasActionsReadPermission = context.inputs.additionalPermissions.get("actions") === "read"; diff --git a/test/branch-cleanup.test.ts b/test/branch-cleanup.test.ts index 488bce8..19ad1a4 100644 --- a/test/branch-cleanup.test.ts +++ b/test/branch-cleanup.test.ts @@ -1,9 +1,9 @@ import { describe, test, expect, beforeEach, afterEach, spyOn } from "bun:test"; -import { checkAndDeleteEmptyBranch } from "../src/github/operations/branch-cleanup"; +import { checkAndCommitOrDeleteBranch } from "../src/github/operations/branch-cleanup"; import type { Octokits } from "../src/github/api/client"; import { GITHUB_SERVER_URL } from "../src/github/api/config"; -describe("checkAndDeleteEmptyBranch", () => { +describe("checkAndCommitOrDeleteBranch", () => { let consoleLogSpy: any; let consoleErrorSpy: any; @@ -43,12 +43,13 @@ describe("checkAndDeleteEmptyBranch", () => { test("should return no branch link and not delete when branch is undefined", async () => { const mockOctokit = createMockOctokit(); - const result = await checkAndDeleteEmptyBranch( + const result = await checkAndCommitOrDeleteBranch( mockOctokit, "owner", "repo", undefined, "main", + false, ); expect(result.shouldDeleteBranch).toBe(false); @@ -56,14 +57,15 @@ describe("checkAndDeleteEmptyBranch", () => { expect(consoleLogSpy).not.toHaveBeenCalled(); }); - test("should delete branch and return no link when branch has no commits", async () => { + test("should mark branch for deletion when commit signing is enabled and no commits", async () => { const mockOctokit = createMockOctokit({ total_commits: 0 }); - const result = await checkAndDeleteEmptyBranch( + const result = await checkAndCommitOrDeleteBranch( mockOctokit, "owner", "repo", "claude/issue-123-20240101_123456", "main", + true, // commit signing enabled ); expect(result.shouldDeleteBranch).toBe(true); @@ -71,19 +73,17 @@ describe("checkAndDeleteEmptyBranch", () => { expect(consoleLogSpy).toHaveBeenCalledWith( "Branch claude/issue-123-20240101_123456 has no commits from Claude, will delete it", ); - expect(consoleLogSpy).toHaveBeenCalledWith( - "✅ Deleted empty branch: claude/issue-123-20240101_123456", - ); }); test("should not delete branch and return link when branch has commits", async () => { const mockOctokit = createMockOctokit({ total_commits: 3 }); - const result = await checkAndDeleteEmptyBranch( + const result = await checkAndCommitOrDeleteBranch( mockOctokit, "owner", "repo", "claude/issue-123-20240101_123456", "main", + false, ); expect(result.shouldDeleteBranch).toBe(false); @@ -109,12 +109,13 @@ describe("checkAndDeleteEmptyBranch", () => { }, } as any as Octokits; - const result = await checkAndDeleteEmptyBranch( + const result = await checkAndCommitOrDeleteBranch( mockOctokit, "owner", "repo", "claude/issue-123-20240101_123456", "main", + false, ); expect(result.shouldDeleteBranch).toBe(false); @@ -131,12 +132,13 @@ describe("checkAndDeleteEmptyBranch", () => { const deleteError = new Error("Delete failed"); const mockOctokit = createMockOctokit({ total_commits: 0 }, deleteError); - const result = await checkAndDeleteEmptyBranch( + const result = await checkAndCommitOrDeleteBranch( mockOctokit, "owner", "repo", "claude/issue-123-20240101_123456", "main", + true, // commit signing enabled - will try to delete ); expect(result.shouldDeleteBranch).toBe(true); diff --git a/test/create-prompt.test.ts b/test/create-prompt.test.ts index 915d3fe..4fd3591 100644 --- a/test/create-prompt.test.ts +++ b/test/create-prompt.test.ts @@ -133,7 +133,7 @@ describe("generatePrompt", () => { }, }; - const prompt = generatePrompt(envVars, mockGitHubData); + const prompt = generatePrompt(envVars, mockGitHubData, false); expect(prompt).toContain("You are Claude, an AI assistant"); expect(prompt).toContain("GENERAL_COMMENT"); @@ -161,7 +161,7 @@ describe("generatePrompt", () => { }, }; - const prompt = generatePrompt(envVars, mockGitHubData); + const prompt = generatePrompt(envVars, mockGitHubData, false); expect(prompt).toContain("PR_REVIEW"); expect(prompt).toContain("true"); @@ -187,7 +187,7 @@ describe("generatePrompt", () => { }, }; - const prompt = generatePrompt(envVars, mockGitHubData); + const prompt = generatePrompt(envVars, mockGitHubData, false); expect(prompt).toContain("ISSUE_CREATED"); expect(prompt).toContain( @@ -215,7 +215,7 @@ describe("generatePrompt", () => { }, }; - const prompt = generatePrompt(envVars, mockGitHubData); + const prompt = generatePrompt(envVars, mockGitHubData, false); expect(prompt).toContain("ISSUE_ASSIGNED"); expect(prompt).toContain( @@ -242,7 +242,7 @@ describe("generatePrompt", () => { }, }; - const prompt = generatePrompt(envVars, mockGitHubData); + const prompt = generatePrompt(envVars, mockGitHubData, false); expect(prompt).toContain("ISSUE_LABELED"); expect(prompt).toContain( @@ -269,7 +269,7 @@ describe("generatePrompt", () => { }, }; - const prompt = generatePrompt(envVars, mockGitHubData); + const prompt = generatePrompt(envVars, mockGitHubData, false); expect(prompt).toContain(""); expect(prompt).toContain("Fix the bug in the login form"); @@ -292,7 +292,7 @@ describe("generatePrompt", () => { }, }; - const prompt = generatePrompt(envVars, mockGitHubData); + const prompt = generatePrompt(envVars, mockGitHubData, false); expect(prompt).toContain("PULL_REQUEST"); expect(prompt).toContain("true"); @@ -317,7 +317,7 @@ describe("generatePrompt", () => { }, }; - const prompt = generatePrompt(envVars, mockGitHubData); + const prompt = generatePrompt(envVars, mockGitHubData, false); expect(prompt).toContain("CUSTOM INSTRUCTIONS:\nAlways use TypeScript"); }); @@ -339,11 +339,12 @@ describe("generatePrompt", () => { }, }; - const prompt = generatePrompt(envVars, mockGitHubData); + const prompt = generatePrompt(envVars, mockGitHubData, false); expect(prompt).toContain("johndoe"); + // With commit signing disabled, co-author info appears in git commit instructions expect(prompt).toContain( - 'Use: "Co-authored-by: johndoe "', + "Co-authored-by: johndoe ", ); }); @@ -360,12 +361,10 @@ describe("generatePrompt", () => { }, }; - const prompt = generatePrompt(envVars, mockGitHubData); + const prompt = generatePrompt(envVars, mockGitHubData, false); - // Should contain PR-specific instructions - expect(prompt).toContain( - "Push directly using mcp__github_file_ops__commit_files to the existing branch", - ); + // Should contain PR-specific instructions (git commands when not using signing) + expect(prompt).toContain("git push"); expect(prompt).toContain( "Always push to the existing branch when triggered on a PR", ); @@ -393,7 +392,7 @@ describe("generatePrompt", () => { }, }; - const prompt = generatePrompt(envVars, mockGitHubData); + const prompt = generatePrompt(envVars, mockGitHubData, false); // Should contain Issue-specific instructions expect(prompt).toContain( @@ -432,7 +431,7 @@ describe("generatePrompt", () => { }, }; - const prompt = generatePrompt(envVars, mockGitHubData); + const prompt = generatePrompt(envVars, mockGitHubData, false); // Should contain the actual branch name with timestamp expect(prompt).toContain( @@ -462,7 +461,7 @@ describe("generatePrompt", () => { }, }; - const prompt = generatePrompt(envVars, mockGitHubData); + const prompt = generatePrompt(envVars, mockGitHubData, false); // Should contain branch-specific instructions like issues expect(prompt).toContain( @@ -500,12 +499,10 @@ describe("generatePrompt", () => { }, }; - const prompt = generatePrompt(envVars, mockGitHubData); + const prompt = generatePrompt(envVars, mockGitHubData, false); - // Should contain open PR instructions - expect(prompt).toContain( - "Push directly using mcp__github_file_ops__commit_files to the existing branch", - ); + // Should contain open PR instructions (git commands when not using signing) + expect(prompt).toContain("git push"); expect(prompt).toContain( "Always push to the existing branch when triggered on a PR", ); @@ -533,7 +530,7 @@ describe("generatePrompt", () => { }, }; - const prompt = generatePrompt(envVars, mockGitHubData); + const prompt = generatePrompt(envVars, mockGitHubData, false); // Should contain new branch instructions expect(prompt).toContain( @@ -561,7 +558,7 @@ describe("generatePrompt", () => { }, }; - const prompt = generatePrompt(envVars, mockGitHubData); + const prompt = generatePrompt(envVars, mockGitHubData, false); // Should contain new branch instructions expect(prompt).toContain( @@ -589,7 +586,7 @@ describe("generatePrompt", () => { }, }; - const prompt = generatePrompt(envVars, mockGitHubData); + const prompt = generatePrompt(envVars, mockGitHubData, false); // Should contain new branch instructions expect(prompt).toContain( @@ -598,6 +595,61 @@ describe("generatePrompt", () => { expect(prompt).toContain("Create a PR](https://github.com/"); expect(prompt).toContain("Reference to the original PR"); }); + + test("should include git commands when useCommitSigning is false", () => { + const envVars: PreparedContext = { + repository: "owner/repo", + claudeCommentId: "12345", + triggerPhrase: "@claude", + eventData: { + eventName: "issue_comment", + commentId: "67890", + isPR: true, + prNumber: "123", + commentBody: "@claude fix the bug", + }, + }; + + const prompt = generatePrompt(envVars, mockGitHubData, false); + + // Should have git command instructions + expect(prompt).toContain("Use git commands via the Bash tool"); + expect(prompt).toContain("git add"); + expect(prompt).toContain("git commit"); + expect(prompt).toContain("git push"); + + // Should use the minimal comment tool + expect(prompt).toContain("mcp__github_comment__update_claude_comment"); + + // Should not have commit signing tool references + expect(prompt).not.toContain("mcp__github_file_ops__commit_files"); + }); + + test("should include commit signing tools when useCommitSigning is true", () => { + const envVars: PreparedContext = { + repository: "owner/repo", + claudeCommentId: "12345", + triggerPhrase: "@claude", + eventData: { + eventName: "issue_comment", + commentId: "67890", + isPR: true, + prNumber: "123", + commentBody: "@claude fix the bug", + }, + }; + + const prompt = generatePrompt(envVars, mockGitHubData, true); + + // Should have commit signing tool instructions + expect(prompt).toContain("mcp__github_file_ops__commit_files"); + expect(prompt).toContain("mcp__github_file_ops__delete_files"); + // Comment tool should always be from comment server, not file ops + expect(prompt).toContain("mcp__github_comment__update_claude_comment"); + + // Should not have git command instructions + expect(prompt).not.toContain("Use git commands via the Bash tool"); + }); }); describe("getEventTypeAndContext", () => { @@ -689,7 +741,7 @@ describe("getEventTypeAndContext", () => { }); describe("buildAllowedToolsString", () => { - test("should return issue comment tool for regular events", () => { + test("should return correct tools for regular events (default no signing)", () => { const result = buildAllowedToolsString(); // The base tools should be in the result @@ -699,15 +751,20 @@ describe("buildAllowedToolsString", () => { expect(result).toContain("LS"); expect(result).toContain("Read"); expect(result).toContain("Write"); - expect(result).toContain("mcp__github_file_ops__update_claude_comment"); - expect(result).not.toContain("mcp__github__update_issue_comment"); - expect(result).not.toContain("mcp__github__update_pull_request_comment"); - expect(result).toContain("mcp__github_file_ops__commit_files"); - expect(result).toContain("mcp__github_file_ops__delete_files"); + + // Default is no commit signing, so should have specific Bash git commands + expect(result).toContain("Bash(git add:*)"); + expect(result).toContain("Bash(git commit:*)"); + expect(result).toContain("Bash(git push:*)"); + expect(result).toContain("mcp__github_comment__update_claude_comment"); + + // Should not have commit signing tools + expect(result).not.toContain("mcp__github_file_ops__commit_files"); + expect(result).not.toContain("mcp__github_file_ops__delete_files"); }); - test("should return PR comment tool for inline review comments", () => { - const result = buildAllowedToolsString(); + test("should return correct tools with default parameters", () => { + const result = buildAllowedToolsString([], false, false); // The base tools should be in the result expect(result).toContain("Edit"); @@ -716,11 +773,15 @@ describe("buildAllowedToolsString", () => { expect(result).toContain("LS"); expect(result).toContain("Read"); expect(result).toContain("Write"); - expect(result).toContain("mcp__github_file_ops__update_claude_comment"); - expect(result).not.toContain("mcp__github__update_issue_comment"); - expect(result).not.toContain("mcp__github__update_pull_request_comment"); - expect(result).toContain("mcp__github_file_ops__commit_files"); - expect(result).toContain("mcp__github_file_ops__delete_files"); + + // Should have specific Bash git commands for non-signing mode + expect(result).toContain("Bash(git add:*)"); + expect(result).toContain("Bash(git commit:*)"); + expect(result).toContain("mcp__github_comment__update_claude_comment"); + + // Should not have commit signing tools + expect(result).not.toContain("mcp__github_file_ops__commit_files"); + expect(result).not.toContain("mcp__github_file_ops__delete_files"); }); test("should append custom tools when provided", () => { @@ -773,6 +834,79 @@ describe("buildAllowedToolsString", () => { expect(result).toContain("mcp__github_ci__get_workflow_run_details"); expect(result).toContain("mcp__github_ci__download_job_log"); }); + + test("should include commit signing tools when useCommitSigning is true", () => { + const result = buildAllowedToolsString([], false, true); + + // Base tools should be present + expect(result).toContain("Edit"); + expect(result).toContain("Glob"); + expect(result).toContain("Grep"); + expect(result).toContain("LS"); + expect(result).toContain("Read"); + expect(result).toContain("Write"); + + // Commit signing tools should be included + expect(result).toContain("mcp__github_file_ops__commit_files"); + expect(result).toContain("mcp__github_file_ops__delete_files"); + // Comment tool should always be from github_comment server + expect(result).toContain("mcp__github_comment__update_claude_comment"); + + // Bash should NOT be included when using commit signing (except in comment tool name) + expect(result).not.toContain("Bash("); + }); + + test("should include specific Bash git commands when useCommitSigning is false", () => { + const result = buildAllowedToolsString([], false, false); + + // Base tools should be present + expect(result).toContain("Edit"); + expect(result).toContain("Glob"); + expect(result).toContain("Grep"); + expect(result).toContain("LS"); + expect(result).toContain("Read"); + expect(result).toContain("Write"); + + // Specific Bash git commands should be included + expect(result).toContain("Bash(git add:*)"); + expect(result).toContain("Bash(git commit:*)"); + expect(result).toContain("Bash(git push:*)"); + expect(result).toContain("Bash(git status:*)"); + expect(result).toContain("Bash(git diff:*)"); + expect(result).toContain("Bash(git log:*)"); + expect(result).toContain("Bash(git rm:*)"); + expect(result).toContain("Bash(git config user.name:*)"); + expect(result).toContain("Bash(git config user.email:*)"); + + // Comment tool from minimal server should be included + expect(result).toContain("mcp__github_comment__update_claude_comment"); + + // Commit signing tools should NOT be included + expect(result).not.toContain("mcp__github_file_ops__commit_files"); + expect(result).not.toContain("mcp__github_file_ops__delete_files"); + }); + + test("should handle all combinations of options", () => { + const customTools = ["CustomTool1", "CustomTool2"]; + const result = buildAllowedToolsString(customTools, true, false); + + // Base tools should be present + expect(result).toContain("Edit"); + expect(result).toContain("Bash(git add:*)"); + + // Custom tools should be included + expect(result).toContain("CustomTool1"); + expect(result).toContain("CustomTool2"); + + // GitHub Actions tools should be included + expect(result).toContain("mcp__github_ci__get_ci_status"); + + // Comment tool from minimal server should be included + expect(result).toContain("mcp__github_comment__update_claude_comment"); + + // Commit signing tools should NOT be included + expect(result).not.toContain("mcp__github_file_ops__commit_files"); + }); }); describe("buildDisallowedToolsString", () => { diff --git a/test/install-mcp-server.test.ts b/test/install-mcp-server.test.ts index c9485bc..7c63fb2 100644 --- a/test/install-mcp-server.test.ts +++ b/test/install-mcp-server.test.ts @@ -34,6 +34,7 @@ describe("prepareMcpConfig", () => { branchPrefix: "", useStickyComment: false, additionalPermissions: new Map(), + useCommitSigning: false, }, }; @@ -44,6 +45,22 @@ describe("prepareMcpConfig", () => { entityNumber: 456, }; + const mockContextWithSigning: ParsedGitHubContext = { + ...mockContext, + inputs: { + ...mockContext.inputs, + useCommitSigning: true, + }, + }; + + const mockPRContextWithSigning: ParsedGitHubContext = { + ...mockPRContext, + inputs: { + ...mockPRContext.inputs, + useCommitSigning: true, + }, + }; + beforeEach(() => { consoleInfoSpy = spyOn(core, "info").mockImplementation(() => {}); consoleWarningSpy = spyOn(core, "warning").mockImplementation(() => {}); @@ -65,7 +82,7 @@ describe("prepareMcpConfig", () => { processExitSpy.mockRestore(); }); - test("should return base config when no additional config is provided and no allowed_tools", async () => { + test("should return comment server when commit signing is disabled", async () => { const result = await prepareMcpConfig({ githubToken: "test-token", owner: "test-owner", @@ -78,6 +95,37 @@ describe("prepareMcpConfig", () => { const parsed = JSON.parse(result); expect(parsed.mcpServers).toBeDefined(); expect(parsed.mcpServers.github).not.toBeDefined(); + expect(parsed.mcpServers.github_file_ops).not.toBeDefined(); + expect(parsed.mcpServers.github_comment).toBeDefined(); + expect(parsed.mcpServers.github_comment.env.GITHUB_TOKEN).toBe( + "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 () => { + const contextWithSigning = { + ...mockContext, + inputs: { + ...mockContext.inputs, + useCommitSigning: true, + }, + }; + + const result = await prepareMcpConfig({ + githubToken: "test-token", + owner: "test-owner", + repo: "test-repo", + branch: "test-branch", + allowedTools: [], + context: contextWithSigning, + }); + + const parsed = JSON.parse(result); + expect(parsed.mcpServers).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.env.GITHUB_TOKEN).toBe( "test-token", @@ -105,13 +153,22 @@ describe("prepareMcpConfig", () => { 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_comment).toBeDefined(); + expect(parsed.mcpServers.github_file_ops).not.toBeDefined(); expect(parsed.mcpServers.github.env.GITHUB_PERSONAL_ACCESS_TOKEN).toBe( "test-token", ); }); test("should not include github MCP server when only file_ops tools are allowed", async () => { + const contextWithSigning = { + ...mockContext, + inputs: { + ...mockContext.inputs, + useCommitSigning: true, + }, + }; + const result = await prepareMcpConfig({ githubToken: "test-token", owner: "test-owner", @@ -121,7 +178,7 @@ describe("prepareMcpConfig", () => { "mcp__github_file_ops__commit_files", "mcp__github_file_ops__update_claude_comment", ], - context: mockContext, + context: contextWithSigning, }); const parsed = JSON.parse(result); @@ -130,7 +187,7 @@ describe("prepareMcpConfig", () => { expect(parsed.mcpServers.github_file_ops).toBeDefined(); }); - test("should include file_ops server even when no GitHub tools are allowed", async () => { + test("should include comment server when no GitHub tools are allowed and signing disabled", async () => { const result = await prepareMcpConfig({ githubToken: "test-token", owner: "test-owner", @@ -143,7 +200,8 @@ describe("prepareMcpConfig", () => { const parsed = JSON.parse(result); expect(parsed.mcpServers).toBeDefined(); expect(parsed.mcpServers.github).not.toBeDefined(); - expect(parsed.mcpServers.github_file_ops).toBeDefined(); + expect(parsed.mcpServers.github_file_ops).not.toBeDefined(); + expect(parsed.mcpServers.github_comment).toBeDefined(); }); test("should return base config when additional config is empty string", async () => { @@ -160,7 +218,7 @@ describe("prepareMcpConfig", () => { const parsed = JSON.parse(result); expect(parsed.mcpServers).toBeDefined(); expect(parsed.mcpServers.github).not.toBeDefined(); - expect(parsed.mcpServers.github_file_ops).toBeDefined(); + expect(parsed.mcpServers.github_comment).toBeDefined(); expect(consoleWarningSpy).not.toHaveBeenCalled(); }); @@ -178,7 +236,7 @@ describe("prepareMcpConfig", () => { const parsed = JSON.parse(result); expect(parsed.mcpServers).toBeDefined(); expect(parsed.mcpServers.github).not.toBeDefined(); - expect(parsed.mcpServers.github_file_ops).toBeDefined(); + expect(parsed.mcpServers.github_comment).toBeDefined(); expect(consoleWarningSpy).not.toHaveBeenCalled(); }); @@ -205,7 +263,7 @@ describe("prepareMcpConfig", () => { "mcp__github__create_issue", "mcp__github_file_ops__commit_files", ], - context: mockContext, + context: mockContextWithSigning, }); const parsed = JSON.parse(result); @@ -243,7 +301,7 @@ describe("prepareMcpConfig", () => { "mcp__github__create_issue", "mcp__github_file_ops__commit_files", ], - context: mockContext, + context: mockContextWithSigning, }); const parsed = JSON.parse(result); @@ -281,7 +339,7 @@ describe("prepareMcpConfig", () => { branch: "test-branch", additionalMcpConfig: additionalConfig, allowedTools: [], - context: mockContext, + context: mockContextWithSigning, }); const parsed = JSON.parse(result); @@ -301,7 +359,7 @@ describe("prepareMcpConfig", () => { branch: "test-branch", additionalMcpConfig: invalidJson, allowedTools: [], - context: mockContext, + context: mockContextWithSigning, }); const parsed = JSON.parse(result); @@ -322,7 +380,7 @@ describe("prepareMcpConfig", () => { branch: "test-branch", additionalMcpConfig: nonObjectJson, allowedTools: [], - context: mockContext, + context: mockContextWithSigning, }); const parsed = JSON.parse(result); @@ -346,7 +404,7 @@ describe("prepareMcpConfig", () => { branch: "test-branch", additionalMcpConfig: nullJson, allowedTools: [], - context: mockContext, + context: mockContextWithSigning, }); const parsed = JSON.parse(result); @@ -370,7 +428,7 @@ describe("prepareMcpConfig", () => { branch: "test-branch", additionalMcpConfig: arrayJson, allowedTools: [], - context: mockContext, + context: mockContextWithSigning, }); const parsed = JSON.parse(result); @@ -417,7 +475,7 @@ describe("prepareMcpConfig", () => { branch: "test-branch", additionalMcpConfig: additionalConfig, allowedTools: [], - context: mockContext, + context: mockContextWithSigning, }); const parsed = JSON.parse(result); @@ -439,7 +497,7 @@ describe("prepareMcpConfig", () => { repo: "test-repo", branch: "test-branch", allowedTools: [], - context: mockContext, + context: mockContextWithSigning, }); const parsed = JSON.parse(result); @@ -460,7 +518,7 @@ describe("prepareMcpConfig", () => { repo: "test-repo", branch: "test-branch", allowedTools: [], - context: mockContext, + context: mockContextWithSigning, }); const parsed = JSON.parse(result); @@ -478,6 +536,7 @@ describe("prepareMcpConfig", () => { inputs: { ...mockPRContext.inputs, additionalPermissions: new Map([["actions", "read"]]), + useCommitSigning: true, }, }; @@ -506,7 +565,7 @@ describe("prepareMcpConfig", () => { repo: "test-repo", branch: "test-branch", allowedTools: [], - context: mockContext, + context: mockContextWithSigning, }); const parsed = JSON.parse(result); @@ -524,7 +583,7 @@ describe("prepareMcpConfig", () => { repo: "test-repo", branch: "test-branch", allowedTools: [], - context: mockPRContext, + context: mockPRContextWithSigning, }); const parsed = JSON.parse(result); diff --git a/test/mockContext.ts b/test/mockContext.ts index 8db88da..d035afc 100644 --- a/test/mockContext.ts +++ b/test/mockContext.ts @@ -22,6 +22,7 @@ const defaultInputs = { branchPrefix: "claude/", useStickyComment: false, additionalPermissions: new Map(), + useCommitSigning: false, }; const defaultRepository = { diff --git a/test/permissions.test.ts b/test/permissions.test.ts index 2fb2443..7471acb 100644 --- a/test/permissions.test.ts +++ b/test/permissions.test.ts @@ -70,6 +70,7 @@ describe("checkWritePermissions", () => { branchPrefix: "claude/", useStickyComment: false, additionalPermissions: new Map(), + useCommitSigning: false, }, }); diff --git a/test/trigger-validation.test.ts b/test/trigger-validation.test.ts index eba2b3c..eaaf834 100644 --- a/test/trigger-validation.test.ts +++ b/test/trigger-validation.test.ts @@ -38,6 +38,7 @@ describe("checkContainsTrigger", () => { branchPrefix: "claude/", useStickyComment: false, additionalPermissions: new Map(), + useCommitSigning: false, }, }); expect(checkContainsTrigger(context)).toBe(true); @@ -68,6 +69,7 @@ describe("checkContainsTrigger", () => { branchPrefix: "claude/", useStickyComment: false, additionalPermissions: new Map(), + useCommitSigning: false, }, }); expect(checkContainsTrigger(context)).toBe(false); @@ -282,6 +284,7 @@ describe("checkContainsTrigger", () => { branchPrefix: "claude/", useStickyComment: false, additionalPermissions: new Map(), + useCommitSigning: false, }, }); expect(checkContainsTrigger(context)).toBe(true); @@ -313,6 +316,7 @@ describe("checkContainsTrigger", () => { branchPrefix: "claude/", useStickyComment: false, additionalPermissions: new Map(), + useCommitSigning: false, }, }); expect(checkContainsTrigger(context)).toBe(true); @@ -344,6 +348,7 @@ describe("checkContainsTrigger", () => { branchPrefix: "claude/", useStickyComment: false, additionalPermissions: new Map(), + useCommitSigning: false, }, }); expect(checkContainsTrigger(context)).toBe(false);