tools: add Claude configuration and PR review command - #371
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds Claude AI integration for automated pull request reviews. It includes comprehensive documentation for Claude in CLAUDE.md with project structure, coding standards, and testing requirements. A GitHub Actions workflow is introduced to automatically trigger Claude reviews, and a custom Claude command is defined to execute the review process.
Changes:
- Restructured and expanded CLAUDE.md with detailed project overview, directory structure, code style guidelines, and testing requirements
- Added .github/workflows/ai.yaml to trigger AI-powered PR reviews via Claude on pull requests and ci_** branch pushes
- Created .claude/commands/review-pr.md defining the PR review process including code review, build, test, style, documentation, and coverage checks
- Updated .gitignore to exclude CLAUDE.local.md for local customizations
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 10 comments.
| File | Description |
|---|---|
| CLAUDE.md | Completely restructured from simple communication guidelines to comprehensive documentation including project overview, directory structure, code style, naming conventions, and testing requirements |
| .gitignore | Added exclusion for CLAUDE.local.md to allow local Claude instruction customizations |
| .github/workflows/ai.yaml | New workflow to trigger Claude PR reviews on pull requests and ci_** branch pushes using Anthropic API |
| .claude/commands/review-pr.md | New Claude command defining comprehensive PR review process with setup, code review, build, test, style, documentation, coverage, and commit validation steps |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| push: | ||
| branches: | ||
| - 'ci_**' |
There was a problem hiding this comment.
The workflow runs on push to 'ci_' branches in addition to pull requests. This might trigger the AI review unnecessarily during CI development on these special branches. Consider whether AI reviews are needed for pushes to ci_ branches, or if they should only run on pull requests.
| push: | |
| branches: | |
| - 'ci_**' |
| # AI PR review | ||
| name: AI PR Review | ||
|
|
||
| on: | ||
| push: | ||
| branches: | ||
| - 'ci_**' | ||
| pull_request: | ||
| types: [opened, synchronize] | ||
|
|
There was a problem hiding this comment.
The workflow doesn't include permissions configuration, unlike the ci.yaml workflow which has a comprehensive permissions block. For security best practices, explicitly define the minimum required permissions for this workflow (at minimum, it may need 'contents: read' and 'pull-requests: write' if the Claude tool posts comments to PRs).
| name: "AI PR review" | ||
| steps: | ||
| - name: Checkout bpfilter | ||
| uses: actions/checkout@v4 |
There was a problem hiding this comment.
The workflow uses actions/checkout@v4 while other workflows in ci.yaml use actions/checkout@v2. This inconsistency could lead to unexpected behavior. Consider using the same version across all workflows for consistency, or ensure there's a specific reason for using v4 here.
| uses: actions/checkout@v4 | |
| uses: actions/checkout@v2 |
| env: | ||
| ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }} | ||
| run: | | ||
| claude /review-pr ${{ github.event.pull_request.number }} No newline at end of file |
There was a problem hiding this comment.
The workflow will fail when triggered by a push event because github.event.pull_request.number will be null/empty for push events. The workflow should either be restricted to only pull_request events, or the command should handle both push and pull_request events appropriately (e.g., by deriving the PR number differently for push events or skipping execution).
| name: "AI PR review" | ||
| steps: | ||
| - name: Checkout bpfilter | ||
| uses: actions/checkout@v4 |
There was a problem hiding this comment.
The workflow attempts to checkout code before installing Node.js and Claude Code. If the command execution at line 27 needs to access repository files (which it likely does based on the review-pr.md content), the checkout step should remain. However, there's a potential issue: the checkout doesn't fetch the full PR context needed for the review. Consider adding fetch-depth: 0 to the checkout action and potentially using ref: refs/pull/${{ github.event.pull_request.number }}/merge to checkout the PR merge state.
| uses: actions/checkout@v4 | |
| uses: actions/checkout@v4 | |
| with: | |
| fetch-depth: 0 | |
| ref: refs/pull/${{ github.event.pull_request.number }}/merge |
| - name: Install Node.js and npm | ||
| run: dnf install -y nodejs npm | ||
| - name: Install Claude Code | ||
| run: npm install -g @anthropic-ai/claude-code |
There was a problem hiding this comment.
The package '@anthropic-ai/claude-code' may not be the correct or official package name for Claude CLI integration. The actual package name should be verified, as this could cause installation failures. Check Anthropic's official documentation for the correct package name to install Claude CLI tools.
| run: npm install -g @anthropic-ai/claude-code | |
| run: npm install -g @anthropic-ai/claude |
| - name: Install Claude Code | ||
| run: npm install -g @anthropic-ai/claude-code | ||
| - name: Review PR | ||
| env: | ||
| ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }} | ||
| run: | | ||
| claude /review-pr ${{ github.event.pull_request.number }} No newline at end of file |
There was a problem hiding this comment.
The command 'claude /review-pr' assumes the Claude CLI is installed and available in PATH, and that it can directly execute custom commands defined in the .claude/commands/ directory. The integration between the npm package installation and the actual Claude CLI invocation needs verification. Additionally, the command may need to be invoked from the repository root directory where the .claude/ directory exists.
| - name: Install Claude Code | |
| run: npm install -g @anthropic-ai/claude-code | |
| - name: Review PR | |
| env: | |
| ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }} | |
| run: | | |
| claude /review-pr ${{ github.event.pull_request.number }} | |
| - name: Review PR | |
| env: | |
| ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }} | |
| working-directory: ${{ github.workspace }} | |
| run: | | |
| npx -y @anthropic-ai/claude-code /review-pr ${{ github.event.pull_request.number }} |
| - Configure: `cmake -S . -B <build_dir> -DCMAKE_BUILD_TYPE=<mode>` | ||
| - `build_dir`: `/tmp/bpfilter-pr-$ARGUMENTS` | ||
| - Use `-DWITH_COVERAGE=1` for coverage information |
There was a problem hiding this comment.
The instructions specify setting build_dir to /tmp/bpfilter-pr-$ARGUMENTS, but then use the generic placeholder <build_dir> in subsequent commands. For clarity, either consistently use the explicit path /tmp/bpfilter-pr-$ARGUMENTS throughout, or add a note explaining that <build_dir> refers to /tmp/bpfilter-pr-$ARGUMENTS.
| make -C <build_dir> integration # Integration tests | ||
| make -C <build_dir> check # Style check and linter | ||
|
|
||
| ctest --test-dir build --output-on-failure -R <pattern> # Run a specific test |
There was a problem hiding this comment.
The command at line 67 refers to 'build' directory instead of using the <build_dir> placeholder that's used elsewhere. This should be <build_dir> for consistency with the rest of the document, or should reference the specific /tmp/bpfilter-pr-$ARGUMENTS path defined earlier.
| ctest --test-dir build --output-on-failure -R <pattern> # Run a specific test | |
| ctest --test-dir <build_dir> --output-on-failure -R <pattern> # Run a specific test |
| env: | ||
| ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }} | ||
| run: | |
There was a problem hiding this comment.
The ANTHROPIC_API_KEY secret is being passed as an environment variable, which is correct. However, ensure that the secret 'ANTHROPIC_API_KEY' has been configured in the repository settings. The workflow will fail if this secret is not set, and there's no error handling or validation to provide a clear error message.
Add configuration for Claude, including a PR review command. This PR also enables PR review by Claude.