Skip to content

POC: Action to invoke Claude Code reviews#61

Open
Mirasii wants to merge 2 commits into
mainfrom
poc/claude-automatic-reviews
Open

POC: Action to invoke Claude Code reviews#61
Mirasii wants to merge 2 commits into
mainfrom
poc/claude-automatic-reviews

Conversation

@Mirasii

@Mirasii Mirasii commented Jul 7, 2026

Copy link
Copy Markdown
Member

No description provided.

@Mirasii

Mirasii commented Jul 7, 2026

Copy link
Copy Markdown
Member Author

/claude-review

@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown

Claude finished @Mirasii's task in 1m 50s —— View job


Claude Code Review

  • Gather context (read workflow file, diff)
  • Understand the request (code review of new workflow)
  • Review for correctness bugs and edge cases
  • Review for security concerns
  • Review for maintainability / readability
  • Review for test coverage gaps
  • Post final review

This PR adds .github/workflows/claude-review.yml, a POC workflow to run Claude Code as an automated PR reviewer. Overall the design is solid: least-privilege permissions: block (no contents: write), a --allowedTools allowlist restricted to read + comment tools, concurrency to cancel superseded runs, and a timeout-minutes cap. A few things worth addressing before/at merge:

Correctness / edge cases

  • concurrency.group (line 44) and the PR NUMBER: prompt line (line 93) both use ${{ github.event.pull_request.number }}. The banner at lines 36–39/51–53 documents swapping the trigger back to issue_comment and re-enabling the issue.number references in the if: guard, but doesn't call out that these two other references also need to change to github.event.issue.number — on an issue_comment event, github.event.pull_request doesn't exist, so the concurrency group would collapse to claude-review- for every comment-triggered run (causing unrelated PRs' reviews to cancel each other) and the prompt would omit the PR number. Worth a grep -n "event.pull_request" pass when reverting.
  • Trigger types (line 33) is [opened, reopened, synchronize], with no ready_for_review. A draft PR marked "ready for review" without a new push won't trigger this test workflow (minor, since it's temporary/test-only per the banner).

Security

  • The intended issue_comment trigger (commented out, lines 38–39, with the guard sketched at lines 51–53) only checks for the /claude-review phrase — there's no author_association check. Once re-enabled, any user able to comment on the PR (including non-collaborators on a public repo) could invoke the action, spending the CLAUDE_CODE_OAUTH_TOKEN quota. Consider gating on github.event.comment.author_association (e.g. OWNER/MEMBER/COLLABORATOR) in addition to the trigger phrase.
  • Both third-party actions are pinned to floating major-version tags (actions/checkout@v5, anthropics/claude-code-action@v1 — lines 69, 74) rather than commit SHAs. Since this workflow has access to a paid OAuth secret and processes PR-controlled content (a classic prompt-injection surface, even if the tool allowlist limits the blast radius to posting a comment), pinning to SHAs would be a worthwhile hardening step.
  • Good use of defense-in-depth: contents: read (no write) plus the --allowedTools allowlist (line 115) means even a successful prompt injection can't get further than posting a comment — no Edit/Write/Bash(git ...) tools are exposed.

Maintainability

  • Line 28 references .github/CLAUDE_REVIEW.md for "automation notes," but that file doesn't exist anywhere in the repo. Either add it or drop the dangling reference.
  • The banner comments are thorough and make the temporary-vs-intended state easy to follow — good practice for a POC that's expected to be revised before merging to main.

Test coverage

  • N/A in the traditional sense (this is a workflow definition, not application code); no actionlint/workflow-validation step exists in the repo, which is fine for a POC of this scope.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant