-
Notifications
You must be signed in to change notification settings - Fork 12
Improve claude review action #77
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,27 +1,39 @@ | ||||||
| name: Claude Code Review with Progress Tracking | ||||||
|
|
||||||
| # Trigger Claude review on PR lifecycle events and explicit mentions | ||||||
|
|
||||||
| on: | ||||||
| # Trigger when a new issue comment is created (for @claude mentions) | ||||||
| issue_comment: | ||||||
| types: [created] | ||||||
| # Trigger when a PR review comment is created/edited/deleted (for @claude mentions) | ||||||
| pull_request_review_comment: | ||||||
| types: [created, edited, deleted] | ||||||
| # Trigger on new or assigned issues (for future extension or automation) | ||||||
| issues: | ||||||
| types: [opened, assigned] | ||||||
| # Trigger when a PR review is submitted (for @claude in the review body) | ||||||
| pull_request_review: | ||||||
| types: [submitted] | ||||||
| # Main trigger for PR events, using pull_request_target for elevated permissions | ||||||
| pull_request_target: | ||||||
| types: [opened, synchronize, reopened] | ||||||
|
|
||||||
| permissions: | ||||||
| # Read repository contents needed for code review | ||||||
| contents: read | ||||||
| # Allow Claude to post review comments on pull requests | ||||||
| pull-requests: write | ||||||
| # Allow Claude to interact with issues if needed | ||||||
| issues: write | ||||||
| # Allow this workflow to manage its own actions if required | ||||||
| actions: write | ||||||
|
|
||||||
| jobs: | ||||||
| claude-review-with-tracking: | ||||||
| runs-on: ubuntu-latest | ||||||
|
|
||||||
| # Only run for trusted authors or when explicitly mentioned by them | ||||||
| if: | | ||||||
| ( | ||||||
| github.event_name == 'pull_request_target' && | ||||||
|
|
@@ -51,30 +63,79 @@ | |||||
| ) | ||||||
|
|
||||||
| steps: | ||||||
| # Checkout the repository at the appropriate commit for review | ||||||
| - name: Checkout repository | ||||||
| uses: actions/checkout@v4 | ||||||
| with: | ||||||
| # Use PR head SHA for pull_request_target, fallback to current SHA otherwise | ||||||
| fetch-depth: 0 | ||||||
| ref: ${{ github.event_name == 'pull_request_target' && github.event.pull_request.head.sha || github.sha }} | ||||||
|
|
||||||
| # Handle fork branches for pull_request_target events | ||||||
| - name: Setup Fork Remote (for pull_request_target) | ||||||
| if: ${{ github.event_name == 'pull_request_target' }} | ||||||
| env: | ||||||
| GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||||||
| run: | | ||||||
| PR_NUMBER=${{ github.event.pull_request.number }} | ||||||
| HEAD_REF="${{ github.event.pull_request.head.ref }}" | ||||||
| HEAD_OWNER="${{ github.event.pull_request.head.repo.owner.login }}" | ||||||
| HEAD_REPO="${{ github.event.pull_request.head.repo.name }}" | ||||||
| CURRENT_OWNER="${{ github.repository_owner }}" | ||||||
|
|
||||||
| # For forked PRs, temporarily change origin URL to fork repository | ||||||
| # This allows claude-code-action to fetch the PR branch correctly | ||||||
|
||||||
| # This allows claude-code-action to fetch the PR branch correctly | |
| # This ensures the correct PR branch is available for claude-code-action to review |
Copilot
AI
Jan 15, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After changing the remote URL to point to the fork, the workflow doesn't restore it back to the original repository. This could cause issues if subsequent steps or actions expect the origin to point to the base repository. Consider adding a step to restore the original remote URL after the Claude review completes, or document why this is intentional.
Copilot
AI
Jan 15, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The variable HEAD_REPO is extracted but never used in this step. It appears to be extracted for consistency with the fork setup logic above, but since it's not referenced anywhere in the subsequent commands, it represents unnecessary work. Consider removing this extraction or documenting why it's needed for future use.
Copilot
AI
Jan 15, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The BASE_BRANCH variable is extracted from the PR metadata but never used in this step. This represents unnecessary processing since the value isn't referenced in any subsequent commands. Consider removing this extraction unless it's needed for future functionality.
| BASE_BRANCH=$(echo "$PR_DATA" | jq -r '.baseRefName') |
Copilot
AI
Jan 15, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment states "This allows claude-code-action to fetch the PR branch correctly" but this is misleading. The git remote manipulation happens in the GitHub Actions runner's context, not within the claude-code-action itself. The action will use the already-checked-out state. Consider revising to: "This ensures the correct PR branch is available for claude-code-action to review".
| # This allows claude-code-action to fetch the PR branch correctly | |
| # This ensures the correct PR branch is available for claude-code-action to review |
Copilot
AI
Jan 15, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After changing the remote URL to point to the fork, the workflow doesn't restore it back to the original repository. This could cause issues if subsequent steps or actions expect the origin to point to the base repository. Consider adding a step to restore the original remote URL after the Claude review completes, or document why this is intentional.
Check failure
Code scanning / CodeQL
Code injection Critical
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@codex Please try to fix the security issue
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary
Testing
View task →