feat: add review mode for automated PR code reviews (#374)

* feat: add review mode for PR code reviews

- Add 'review' as a new execution mode in action.yml
- Use default GitHub Action token (ACTIONS_TOKEN) for review mode
- Create review mode implementation with GitHub MCP tools included by default
- Move review-specific prompt to review mode's generatePrompt method
- Add comprehensive review workflow instructions for inline comments
- Fix type safety with proper mode validation
- Keep agent mode's simple inline prompt handling

* docs: add review mode example workflow

* update sample workflow

* fix: update review mode example to use @beta tag

* fix: enable automatic triggering for review mode on PR events

* fix: export allowed tools environment variables in review mode

The GitHub MCP tools were not being properly allowed because review mode
wasn't exporting the ALLOWED_TOOLS environment variable like agent mode does.
This caused all GitHub MCP tool calls to be blocked with permission errors.

* feat: add review mode workflow for testing

* fix: use INPUT_ prefix for allowed/disallowed tools environment variables

The base action expects INPUT_ALLOWED_TOOLS and INPUT_DISALLOWED_TOOLS
(following GitHub Actions input naming convention) but we were exporting
them without the INPUT_ prefix. This was causing the tools to not be
properly allowed in the base action.

* fix: add explicit review tool names and additional workflow permissions

- Add explicit tool names in case wildcards aren't working properly
- Add statuses and checks write permissions to workflow
- Include both github and github_comment MCP server tools

* refactor: consolidate review workflows and use review mode

- Update claude-review.yml to use review mode instead of direct_prompt
- Use km-anthropic fork action
- Remove duplicate claude-review-mode.yml workflow
- Add synchronize event to review PR updates
- Update permissions for review mode (remove id-token, add pull-requests/issues write)

* feat: enhance review mode to provide detailed tracking comment summary

- Update review mode prompt to explicitly request detailed summaries
- Include issue counts, key findings, and recommendations in tracking comment
- Ensure users can see complete review overview without checking each inline comment

* Revert "refactor: consolidate review workflows and use review mode"

This reverts commit 54ca948599.

* fix: address PR review feedback for review mode

- Make generatePrompt required in Mode interface
- Implement generatePrompt in all modes (tag, agent, review)
- Remove unnecessary git/branch operations from review mode
- Restrict review mode triggers to specific PR actions
- Fix type safety issues by removing any types
- Update tests to support new Mode interface

* test: update mode registry tests to include review mode

* chore: run prettier formatting

* fix: make mode parameter required in generatePrompt function

Remove optional mode parameter since the function throws an error when mode is not provided. This makes the type signature consistent with the actual behavior.

* fix: remove last any type and update README with review mode

- Remove any type cast in review mode by using isPullRequestEvent type guard
- Add review mode documentation to README execution modes section
- Update mode parameter description in README configuration table

* mandatory bun format

* fix: improve review mode GitHub suggestion format instructions

- Add clear guidance on GitHub's suggestion block format
- Emphasize that suggestions must only replace the specific commented lines
- Add examples of correct vs incorrect suggestion formatting
- Clarify when to use multi-line comments with startLine and line parameters
- Guide on handling complex changes that require multiple modifications

This should resolve issues where suggestions aren't directly committable.

* Add missing MCP tools for experimental-review mode based on test requirements

* chore: format code

* docs: add experimental-review mode documentation with clear warnings

* docs: remove emojis from experimental-review mode documentation

* docs: clarify experimental-review mode triggers - depends on workflow configuration

* minor format update

* test: fix registry tests for experimental-review mode name change

* refactor: clean up review mode implementation based on feedback

- Remove unused parameters from generatePrompt in agent and review modes
- Keep Claude comment requirement for review mode (tracking comment)
- Add overridePrompt support to review mode
- Remove non-existent MCP tools from review mode allowed list
- Fix unused import in agent mode

These changes address all review feedback while maintaining clean code
and proper functionality.

* fix: remove redundant update_claude_comment from review mode allowed tools

The github_comment server is always included automatically, so we don't
need to explicitly list mcp__github_comment__update_claude_comment in
the allowed tools.

* feat: review mode now uses review body instead of tracking comment

- Remove tracking comment creation from review mode
- Update prompt to instruct Claude to write comprehensive review in body
- Remove comment ID requirement for review mode
- The review submission body now serves as the main review content

This makes review mode cleaner with one less comment on the PR. The
review body contains all the information that would have been in the
tracking comment.

* add back id-token: write for example

* Add PR number for context + make it mandatory to have a PR associated

* add `mcp__github__add_issue_comment` tool

* rename token

* bun format

---------

Co-authored-by: km-anthropic <km-anthropic@users.noreply.github.com>
This commit is contained in:
km-anthropic
2025-08-01 15:04:23 -07:00
committed by GitHub
parent 0e5fbc0d44
commit 56179f5fc9
12 changed files with 604 additions and 91 deletions

View File

@@ -3,13 +3,37 @@
import { describe, test, expect } from "bun:test";
import {
generatePrompt,
generateDefaultPrompt,
getEventTypeAndContext,
buildAllowedToolsString,
buildDisallowedToolsString,
} from "../src/create-prompt";
import type { PreparedContext } from "../src/create-prompt";
import type { Mode } from "../src/modes/types";
describe("generatePrompt", () => {
// Create a mock tag mode that uses the default prompt
const mockTagMode: Mode = {
name: "tag",
description: "Tag mode",
shouldTrigger: () => true,
prepareContext: (context) => ({ mode: "tag", githubContext: context }),
getAllowedTools: () => [],
getDisallowedTools: () => [],
shouldCreateTrackingComment: () => true,
generatePrompt: (context, githubData, useCommitSigning) =>
generateDefaultPrompt(context, githubData, useCommitSigning),
prepare: async () => ({
commentId: 123,
branchInfo: {
baseBranch: "main",
currentBranch: "main",
claudeBranch: undefined,
},
mcpConfig: "{}",
}),
};
const mockGitHubData = {
contextData: {
title: "Test PR",
@@ -133,7 +157,7 @@ describe("generatePrompt", () => {
},
};
const prompt = generatePrompt(envVars, mockGitHubData, false);
const prompt = generatePrompt(envVars, mockGitHubData, false, mockTagMode);
expect(prompt).toContain("You are Claude, an AI assistant");
expect(prompt).toContain("<event_type>GENERAL_COMMENT</event_type>");
@@ -161,7 +185,7 @@ describe("generatePrompt", () => {
},
};
const prompt = generatePrompt(envVars, mockGitHubData, false);
const prompt = generatePrompt(envVars, mockGitHubData, false, mockTagMode);
expect(prompt).toContain("<event_type>PR_REVIEW</event_type>");
expect(prompt).toContain("<is_pr>true</is_pr>");
@@ -187,7 +211,7 @@ describe("generatePrompt", () => {
},
};
const prompt = generatePrompt(envVars, mockGitHubData, false);
const prompt = generatePrompt(envVars, mockGitHubData, false, mockTagMode);
expect(prompt).toContain("<event_type>ISSUE_CREATED</event_type>");
expect(prompt).toContain(
@@ -215,7 +239,7 @@ describe("generatePrompt", () => {
},
};
const prompt = generatePrompt(envVars, mockGitHubData, false);
const prompt = generatePrompt(envVars, mockGitHubData, false, mockTagMode);
expect(prompt).toContain("<event_type>ISSUE_ASSIGNED</event_type>");
expect(prompt).toContain(
@@ -242,7 +266,7 @@ describe("generatePrompt", () => {
},
};
const prompt = generatePrompt(envVars, mockGitHubData, false);
const prompt = generatePrompt(envVars, mockGitHubData, false, mockTagMode);
expect(prompt).toContain("<event_type>ISSUE_LABELED</event_type>");
expect(prompt).toContain(
@@ -269,7 +293,7 @@ describe("generatePrompt", () => {
},
};
const prompt = generatePrompt(envVars, mockGitHubData, false);
const prompt = generatePrompt(envVars, mockGitHubData, false, mockTagMode);
expect(prompt).toContain("<direct_prompt>");
expect(prompt).toContain("Fix the bug in the login form");
@@ -292,7 +316,7 @@ describe("generatePrompt", () => {
},
};
const prompt = generatePrompt(envVars, mockGitHubData, false);
const prompt = generatePrompt(envVars, mockGitHubData, false, mockTagMode);
expect(prompt).toContain("<event_type>PULL_REQUEST</event_type>");
expect(prompt).toContain("<is_pr>true</is_pr>");
@@ -317,7 +341,7 @@ describe("generatePrompt", () => {
},
};
const prompt = generatePrompt(envVars, mockGitHubData, false);
const prompt = generatePrompt(envVars, mockGitHubData, false, mockTagMode);
expect(prompt).toContain("CUSTOM INSTRUCTIONS:\nAlways use TypeScript");
});
@@ -336,7 +360,7 @@ describe("generatePrompt", () => {
},
};
const prompt = generatePrompt(envVars, mockGitHubData, false);
const prompt = generatePrompt(envVars, mockGitHubData, false, mockTagMode);
expect(prompt).toBe("Simple prompt for owner/repo PR #123");
expect(prompt).not.toContain("You are Claude, an AI assistant");
@@ -371,7 +395,7 @@ describe("generatePrompt", () => {
},
};
const prompt = generatePrompt(envVars, mockGitHubData, false);
const prompt = generatePrompt(envVars, mockGitHubData, false, mockTagMode);
expect(prompt).toContain("Repository: test/repo");
expect(prompt).toContain("PR: 456");
@@ -418,7 +442,7 @@ describe("generatePrompt", () => {
},
};
const prompt = generatePrompt(envVars, issueGitHubData, false);
const prompt = generatePrompt(envVars, issueGitHubData, false, mockTagMode);
expect(prompt).toBe("Issue #789: Bug: Login form broken in owner/repo");
});
@@ -438,7 +462,7 @@ describe("generatePrompt", () => {
},
};
const prompt = generatePrompt(envVars, mockGitHubData, false);
const prompt = generatePrompt(envVars, mockGitHubData, false, mockTagMode);
expect(prompt).toBe("PR: 123, Issue: , Comment: ");
});
@@ -458,7 +482,7 @@ describe("generatePrompt", () => {
},
};
const prompt = generatePrompt(envVars, mockGitHubData, false);
const prompt = generatePrompt(envVars, mockGitHubData, false, mockTagMode);
expect(prompt).toContain("You are Claude, an AI assistant");
expect(prompt).toContain("<event_type>ISSUE_CREATED</event_type>");
@@ -481,7 +505,7 @@ describe("generatePrompt", () => {
},
};
const prompt = generatePrompt(envVars, mockGitHubData, false);
const prompt = generatePrompt(envVars, mockGitHubData, false, mockTagMode);
expect(prompt).toContain("<trigger_username>johndoe</trigger_username>");
// With commit signing disabled, co-author info appears in git commit instructions
@@ -503,7 +527,7 @@ describe("generatePrompt", () => {
},
};
const prompt = generatePrompt(envVars, mockGitHubData, false);
const prompt = generatePrompt(envVars, mockGitHubData, false, mockTagMode);
// Should contain PR-specific instructions (git commands when not using signing)
expect(prompt).toContain("git push");
@@ -534,7 +558,7 @@ describe("generatePrompt", () => {
},
};
const prompt = generatePrompt(envVars, mockGitHubData, false);
const prompt = generatePrompt(envVars, mockGitHubData, false, mockTagMode);
// Should contain Issue-specific instructions
expect(prompt).toContain(
@@ -573,7 +597,7 @@ describe("generatePrompt", () => {
},
};
const prompt = generatePrompt(envVars, mockGitHubData, false);
const prompt = generatePrompt(envVars, mockGitHubData, false, mockTagMode);
// Should contain the actual branch name with timestamp
expect(prompt).toContain(
@@ -603,7 +627,7 @@ describe("generatePrompt", () => {
},
};
const prompt = generatePrompt(envVars, mockGitHubData, false);
const prompt = generatePrompt(envVars, mockGitHubData, false, mockTagMode);
// Should contain branch-specific instructions like issues
expect(prompt).toContain(
@@ -641,7 +665,7 @@ describe("generatePrompt", () => {
},
};
const prompt = generatePrompt(envVars, mockGitHubData, false);
const prompt = generatePrompt(envVars, mockGitHubData, false, mockTagMode);
// Should contain open PR instructions (git commands when not using signing)
expect(prompt).toContain("git push");
@@ -672,7 +696,7 @@ describe("generatePrompt", () => {
},
};
const prompt = generatePrompt(envVars, mockGitHubData, false);
const prompt = generatePrompt(envVars, mockGitHubData, false, mockTagMode);
// Should contain new branch instructions
expect(prompt).toContain(
@@ -700,7 +724,7 @@ describe("generatePrompt", () => {
},
};
const prompt = generatePrompt(envVars, mockGitHubData, false);
const prompt = generatePrompt(envVars, mockGitHubData, false, mockTagMode);
// Should contain new branch instructions
expect(prompt).toContain(
@@ -728,7 +752,7 @@ describe("generatePrompt", () => {
},
};
const prompt = generatePrompt(envVars, mockGitHubData, false);
const prompt = generatePrompt(envVars, mockGitHubData, false, mockTagMode);
// Should contain new branch instructions
expect(prompt).toContain(
@@ -752,7 +776,7 @@ describe("generatePrompt", () => {
},
};
const prompt = generatePrompt(envVars, mockGitHubData, false);
const prompt = generatePrompt(envVars, mockGitHubData, false, mockTagMode);
// Should have git command instructions
expect(prompt).toContain("Use git commands via the Bash tool");
@@ -781,7 +805,7 @@ describe("generatePrompt", () => {
},
};
const prompt = generatePrompt(envVars, mockGitHubData, true);
const prompt = generatePrompt(envVars, mockGitHubData, true, mockTagMode);
// Should have commit signing tool instructions
expect(prompt).toContain("mcp__github_file_ops__commit_files");