Skip to content

Add Claude GitHub action #15711

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 25, 2025
Merged

Add Claude GitHub action #15711

merged 1 commit into from
Jun 25, 2025

Conversation

minimalsm
Copy link
Contributor

@CodeRabbit please review

Copy link

netlify bot commented Jun 24, 2025

Deploy Preview for ethereumorg ready!

Name Link
🔨 Latest commit 433c43a
🔍 Latest deploy log https://app.netlify.com/projects/ethereumorg/deploys/685ab135377e330007da008c
😎 Deploy Preview https://deploy-preview-15711--ethereumorg.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
7 paths audited
Performance: 48 (🔴 down 10 from production)
Accessibility: 95 (🟢 up 1 from production)
Best Practices: 91 (🔴 down 8 from production)
SEO: 99 (no change from production)
PWA: 59 (no change from production)
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify project configuration.

@github-actions github-actions bot added the tooling 🔧 Changes related to tooling of the project label Jun 24, 2025
@minimalsm
Copy link
Contributor Author

minimalsm commented Jun 24, 2025

@coderabbitai please review

Copy link
Contributor

coderabbitai bot commented Jun 24, 2025

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Get PR information including title, description, and files changed
gh pr view 15711 --repo ethereum/ethereum-org-website --json title,body,files,additions,deletions,changedFiles

# Get the actual diff to review the changes
gh pr diff 15711 --repo ethereum/ethereum-org-website

Length of output: 2209


@minimalsm Here's my review of the Claude GitHub Actions integration:

Overall Assessment

The workflow is well-structured and follows GitHub Actions best practices for the most part. However, there are several areas for improvement:

Issues & Recommendations

🔒 Security & Permissions

  • Good: Uses secrets.ANTHROPIC_API_KEY for secure API key handling
  • Good: Permissions are explicitly defined and follow least-privilege principle
  • Consider: The id-token: write permission might not be necessary unless Claude requires OIDC authentication

🛠 Maintainability Concerns

  1. Hardcoded user list: The user allowlist is embedded in the workflow condition:

    contains('minimalsm,pettinarip,wackerow,corwintines', github.event.comment.user.login)

    Recommendation: Consider using GitHub team membership or organization roles instead.

  2. Beta version dependency: Using @beta for production:

    uses: anthropics/claude-code-action@beta

    Recommendation: Pin to a specific version or use @v1 when stable.

📝 Code Quality

  • Missing newline: File should end with a newline character (common convention)
  • Complex conditions: The if condition is quite long and repetitive

🤖 Integration Concerns

Since this repository already uses CodeRabbit for AI-assisted reviews, having both Claude and CodeRabbit might:

  • Create conflicting suggestions
  • Confuse contributors about which AI assistant to use
  • Generate unnecessary noise in PR discussions

🔧 Suggested Improvements

name: Claude PR Assistant

on:
  issue_comment:
    types: [created]
  pull_request_review_comment:
    types: [created]
  issues:
    types: [opened, assigned]
  pull_request_review:
    types: [submitted]

jobs:
  claude-code-action:
    if: |
      contains(github.event.comment.body || github.event.review.body || github.event.issue.body, '@claude') &&
      contains('minimalsm,pettinarip,wackerow,corwintines', github.actor)
    runs-on: ubuntu-latest
    permissions:
      contents: read
      pull-requests: read
      issues: read
    steps:
      - name: Checkout repository
        uses: actions/checkout@v4
        with:
          fetch-depth: 1

      - name: Run Claude PR Action
        uses: anthropics/claude-code-action@v1  # Use stable version when available
        with:
          anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }}
          timeout_minutes: "60"

Questions for Consideration

  1. How will Claude and CodeRabbit coexist? Should there be guidelines for when to use each?
  2. Would it be better to use GitHub team membership for access control?
  3. Is there a plan to migrate from @beta to a stable release?

The workflow is functional and secure, but addressing these maintainability concerns will make it more robust for long-term use.

Copy link
Contributor

coderabbitai bot commented Jun 24, 2025

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

@minimalsm I'll review this PR for you. Let me first gather information about the changes made.


🏁 Script executed:

#!/bin/bash
# Get information about the PR and recent commits
echo "=== PR Information ==="
gh pr view 15711 --json title,body,headRefName,baseRefName,author,files,additions,deletions

echo -e "\n=== Recent commits ==="
git log --oneline -10

echo -e "\n=== Files changed in this PR ==="
gh pr diff 15711 --name-only

echo -e "\n=== Detailed diff ==="
gh pr diff 15711

Length of output: 2592


Great! I've reviewed your PR that adds Claude AI integration to the ethereum-org-website repository. Here's my comprehensive review:

Strengths

  1. Proper Security Controls: Good use of user allowlist to restrict who can trigger Claude
  2. Appropriate Permissions: Well-scoped permissions (read-only for most, id-token write for authentication)
  3. Secret Management: Correctly using GitHub secrets for the Anthropic API key
  4. Comprehensive Triggers: Covers all relevant GitHub events where Claude assistance might be needed
  5. Reasonable Timeout: 60-minute timeout prevents runaway processes

🔧 Recommendations for Improvement

1. File Formatting

# Add a trailing newline at the end of the file
          timeout_minutes: "60"
# <- Add newline here

2. Enhanced User Management

Consider using GitHub teams or organization membership instead of hardcoded usernames:

# More maintainable approach
if: |
  github.actor_id && (
    github.repository_owner == github.actor ||
    contains(fromJSON('["OWNER", "MEMBER"]'), github.event.sender.type)
  )

3. Improved Context for Claude

Consider increasing fetch depth for better code context:

- name: Checkout repository
  uses: actions/checkout@v4
  with:
    fetch-depth: 0  # or a higher number like 50

4. Beta Action Consideration

The anthropics/claude-code-action@beta is in beta. Consider:

  • Pinning to a specific commit SHA for stability
  • Adding error handling for when the action fails
  • Having a fallback mechanism

5. Add Error Handling

- name: Run Claude PR Action
  uses: anthropics/claude-code-action@beta
  with:
    anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }}
    timeout_minutes: "60"
  continue-on-error: true  # Prevents workflow failure from blocking other processes

🚨 Minor Issues

  1. Missing Newline: The file should end with a newline character
  2. Hardcoded User List: Makes maintenance harder when team changes occur

📋 Questions

  1. Have you tested this workflow in a fork or staging environment?
  2. Is the ANTHROPIC_API_KEY secret already configured in the repository settings?
  3. Should there be rate limiting to prevent API quota exhaustion?

Overall Assessment

This is a solid implementation of Claude integration! The workflow is well-thought-out with proper security measures. The main improvements are around maintainability and robustness. Once the trailing newline is added and you consider the user management approach, this should work great for the team.

🐰 Looks pretty good to me! Just a few small tweaks and you'll be all set.

@pettinarip
Copy link
Member

@claude review this PR

@minimalsm minimalsm merged commit 08df855 into dev Jun 25, 2025
7 checks passed
@minimalsm minimalsm deleted the claude-action branch June 25, 2025 09:36
@noelportillo
Copy link

MY NFT

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tooling 🔧 Changes related to tooling of the project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants