Conversation
andrewhsu
added
the
claude-review
andrewhsu
force-pushed
the
DEVPROD-3766-claude-code-review
branch
from
c6bf004 to
5a3de0e
Compare
andrewhsu
added
claude-review
andrewhsu
force-pushed
the
DEVPROD-3766-claude-code-review
branch
3 times, most recently
from
d0a651f to
f23fb2e
Compare
andrewhsu
removed
the
claude-review
andrewhsu
force-pushed
the
DEVPROD-3766-claude-code-review
branch
2 times, most recently
from
6e91451 to
60046cc
Compare
andrewhsu
added
the
claude-review
andrewhsu
force-pushed
the
DEVPROD-3766-claude-code-review
branch
from
60046cc to
cf9aee6
Compare
andrewhsu
added
claude-review
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| actions: read | ||
| claude_args: > | ||
| --model opus | ||
| --max-turns 30 |
There was a problem hiding this comment.
for reference: this claude-code review took 17 turns
andrewhsu
removed
the
claude-review
andrewhsu
added
the
claude-review
andrewhsu
added
claude-review
The job was always skipped because github.event.label is only populated on 'labeled' events. Changed to check PR labels array and added 'labeled' event type to trigger on label additions. Co-Authored-By: Claude Opus 4.5 <[email protected]>
andrewhsu
added
claude-review
Only run workflow when label is applied to reduce extra reviews on intermediate changes. Also need to disable `track_progress` because the feature only works on events opened, synchronize, ready_for_review, reopened. See: https://github.com/anthropics/claude-code-action/blob/v1.0.43/action.yml#L108-L111
andrewhsu
force-pushed
the
DEVPROD-3766-claude-code-review
branch
from
0a61c5d to
3f5db03
Compare
andrewhsu
added
claude-review
| id-token: write | ||
| actions: read | ||
| steps: | ||
| - uses: actions/checkout@v6 |
There was a problem hiding this comment.
Consistency suggestion: Other workflows in this repository use actions/checkout@v4, but this workflow uses @v6. Consider using @v4 for consistency with other workflows, or update all workflows together if migrating to v6.
| - uses: actions/checkout@v6 | |
| - uses: actions/checkout@v4 |
There was a problem hiding this comment.
v6 is intentional to use latest supported version of that action and to get onto node24 before node20 eol
| - run: gh auth setup-git | ||
| env: | ||
| GH_TOKEN: ${{ github.token }} | ||
| - uses: anthropics/claude-code-action@v1 |
There was a problem hiding this comment.
Version pinning suggestion: Using @v1 will automatically get new minor/patch versions, which could introduce unexpected behavior changes. Consider pinning to a specific version (e.g., @v1.0.43 as referenced in the PR description) for more predictable behavior, especially since this is a cost-sensitive workflow.
| - uses: anthropics/claude-code-action@v1 | |
| - uses: anthropics/claude-code-action@v1.0.43 |
There was a problem hiding this comment.
this action updates frequently with bug fixes and i trust anthropic's adherence to semver major version so intentionally used float version v1
| jobs: | ||
| claude-review: | ||
| if: github.event.label.name == 'claude-review' | ||
| runs-on: ubuntu-24.04 |
There was a problem hiding this comment.
Best practice: Consider using ubuntu-latest instead of ubuntu-24.04 for better maintainability. This is consistent with other workflows in the repository (e.g., lint-python.yml, lint-cpp.yml) and avoids needing to update the workflow when newer runners become available.
| runs-on: ubuntu-24.04 | |
| runs-on: ubuntu-latest |
There was a problem hiding this comment.
next ubuntu release may have breaking changes like ubuntu-24.04 did from prev release, so conservatively pegging to 24.04
| permissions: | ||
| contents: read | ||
| pull-requests: write | ||
| id-token: write |
There was a problem hiding this comment.
Security consideration: The id-token: write permission is typically used for OIDC token authentication (e.g., with cloud providers). If use_commit_signing doesn't require this permission, consider removing it to follow the principle of least privilege. If it is required for commit signing, this is fine.
Note: Please verify whether this permission is actually needed by the claude-code-action.
| - run: gh auth setup-git | ||
| env: | ||
| GH_TOKEN: ${{ github.token }} |
There was a problem hiding this comment.
Potential cleanup: Since persist-credentials: false is set on checkout (line 19) and the claude-code-action likely handles its own git authentication, this gh auth setup-git step may be unnecessary. The action's documentation should clarify whether this step is required.
Consider testing without this step to simplify the workflow.
| --disallowed-tools "WebFetch,WebSearch" | ||
| --allowed-tools " | ||
| mcp__github_inline_comment__create_inline_comment, | ||
| mcp__github_ci__get_ci_status, | ||
| mcp__github_ci__get_workflow_run_details, | ||
| mcp__github_ci__download_job_log, | ||
| Bash(gh issue *), | ||
| Bash(gh pr *), | ||
| Bash(gh search *)" |
There was a problem hiding this comment.
Security consideration: The allowed Bash commands (gh issue *, gh pr *, gh search *) are quite permissive. While these are read operations, some gh pr subcommands can be write operations (e.g., gh pr comment, gh pr edit, gh pr merge).
If the intent is read-only access, consider being more specific:
Bash(gh issue view *)Bash(gh pr view *)Bash(gh pr diff *)Bash(gh search issues *)Bash(gh search prs *)
However, if write access to PRs is intentional (e.g., for posting review comments via gh pr comment), this is acceptable. The PR description mentions Claude will "comment inline" which may utilize these permissions.
There was a problem hiding this comment.
intentional to allow comment inline
| prompt: | | ||
| REPO: ${{ github.repository }} | ||
| PR NUMBER: ${{ github.event.pull_request.number }} | ||
| WORKFLOW RUN URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} | ||
|
|
||
| Please review this pull request with a focus on: | ||
| - Code quality and best practices | ||
| - Potential bugs or issues | ||
| - Security implications | ||
| - Performance considerations | ||
|
|
||
| Provide detailed feedback using inline comments for specific issues. | ||
| Include a link to workflow run URL in the final review comment. |
There was a problem hiding this comment.
Enhancement suggestion: The prompt could be more specific to the Redpanda codebase by referencing the CLAUDE.md file guidelines. This would help ensure reviews align with project-specific standards (C++ coding guidelines, naming conventions, Seastar patterns, etc.).
For example, adding a line like:
Follow the coding guidelines documented in CLAUDE.md when reviewing.
This would make the AI reviewer more effective at catching project-specific issues like:
- Missing
ss::namespace prefix for Seastar types - Use of
std::vectorwherechunked_vectorshould be used - Missing
format_tomember functions instead ofoperator<<overloads
|
i've resolved discussion with #29488 (comment) i also updated PR description to better describe behavior of the ready for human review. |
2 participants
jira: DEVPROD-3766
This PR adds a github workflow to run on PRs with github label claude-review. The workflow is triggered only when
claude-reviewlabel is added (for cost control). So, to re-review updates to the PR you will need to remove the label and add the label again.I've gone through several rounds of code review feedback with
claude-reviewand i've resolved the older inline discussions and left the last review inline discussions open, starting from #29488 (review), to show how it reviews. For comparison, the copilot review: #29488 (review)Notable behavior:
opusmodelReference:
Backports Required
Release Notes