Skip to content

Single comprehensive-review workflow on @claude review trigger#226

Merged
dakra merged 1 commit into
mainfrom
claude-review-comprehensive-only
May 3, 2026
Merged

Single comprehensive-review workflow on @claude review trigger#226
dakra merged 1 commit into
mainfrom
claude-review-comprehensive-only

Conversation

@dakra
Copy link
Copy Markdown
Owner

@dakra dakra commented May 3, 2026

Summary

  • Drop the broken pull_request_target review workflow — the Anthropic OIDC backend rejects tokens from pull_request_target events (action#713), which is why the label-triggered review on PR Wrap shell-integration OSCs in tmux DCS-passthrough #197 failed.
  • Drop the freeform @claude mention handler — not needed; the only desired use case is comprehensive PR reviews.
  • Replace both with a single issue_comment-triggered workflow that runs the comprehensive review when a maintainer comments anything containing both @claude and review (catches @claude review, @claude please review, Hey @claude can you review this PR?, etc.).

How it works

  • Trigger: any maintainer comment on a PR containing both @claude and review.
  • Gate: author_association must be OWNER/MEMBER/COLLABORATOR. Outside contributors can't invoke it.
  • Identity: posts as claude[bot] via OAuth token, not the user's account.
  • Model: pinned to claude-opus-4-7.
  • Output: structured comprehensive review with progress tracking and inline diff comments (adapted from the action's official pr-review-comprehensive.yml example).

Test plan

  • Merge to main, then comment @claude review on a follow-up PR and verify the workflow fires.
  • Verify the bot posts as claude[bot].
  • Verify a comment from a non-maintainer with @claude review is filtered out (no workflow run).
  • Verify the model used is Opus 4.7 (visible in the action log / response).

@dakra
Copy link
Copy Markdown
Owner Author

dakra commented May 3, 2026

@claude review

@claude
Copy link
Copy Markdown

claude Bot commented May 3, 2026

Claude finished @dakra's task in 1m 17s —— View job


PR Review

  • Gather context and read changed files
  • Analyze workflow trigger and security gate
  • Review YAML structure and correctness
  • Post comprehensive review

The motivation is sound — switching away from pull_request_target to avoid the Anthropic OIDC rejection is the right call. The overall structure is clean. A few things worth addressing before merge:


Bug: issues: write permission was dropped but is needed

.github/workflows/claude-code-review.yml lines 17–20 — the issues: write permission was removed, but track_progress: true posts a progress-tracking comment to the PR thread. In GitHub's API model, PR thread comments (non-review comments) are issue comments — they require issues: write. Without it, the track_progress step will likely get a 403.

permissions:
  contents: read
  pull-requests: write
  issues: write    # ← needed for track_progress: true to post PR thread comments
  id-token: write

Fix this →


Potential issue: actions/checkout@v6 does not exist

Line 23 — the old workflow used actions/checkout@v4 (the current stable tag). Bumping to @v6 without a corresponding release will silently resolve to an unexpected ref or fail outright. As of now, the latest stable tag is v4. Revert to v4 (or verify v6 actually exists before merging).

Fix this →


Minor: persist-credentials: false removed

The old workflow had persist-credentials: false in the checkout step as a hardening measure (prevents the GITHUB_TOKEN from being written to .git/config where it could be exfiltrated). The new workflow omits it. Since the review action only reads the repo and posts comments, the risk is low — but it's worth keeping as a default-deny hygiene practice.


Good things

  • Security gate is solid: Using author_association (OWNER/MEMBER/COLLABORATOR) is the correct idiomatic approach — it's set by GitHub itself, not user-controlled.
  • Prompt is injection-safe: Only github.repository and github.event.issue.number are interpolated; the comment body is not, avoiding prompt injection from untrusted comment text.
  • issue_comment correctly resolves the pull_request_target OIDC problem: The OIDC token from issue_comment events is not subject to the same backend restrictions.
  • github.event.issue.number is correct for the issue_comment trigger — PRs are issues in GitHub's model, so this yields the PR number as expected.
  • Dropping claude.yml: Removing the broad freeform handler is a clean simplification given the stated use case.

Summary: Two items to fix before merging — add issues: write back to permissions (it's needed by track_progress), and verify/revert actions/checkout@v6. Everything else looks good.

@dakra dakra force-pushed the claude-review-comprehensive-only branch from 3d55b93 to b596e9e Compare May 3, 2026 14:58
Drop the broken pull_request_target review workflow and the freeform
@claude mention handler. Replace with a single issue_comment trigger
that runs a comprehensive code review when a maintainer comments
'@claude review' (or any comment containing both '@claude' and 'review').

The pull_request_target trigger fails the OIDC token exchange against
the Anthropic backend (see anthropics/claude-code-action#713). The
issue_comment event is allowlisted and works for both same-repo and
fork PRs.

Pin model to claude-opus-4-7. Use track_progress and the inline-comment
tool allowlist from the action's pr-review-comprehensive example.
@dakra dakra force-pushed the claude-review-comprehensive-only branch from b596e9e to 3a64a1f Compare May 3, 2026 15:19
@dakra dakra merged commit 3a64a1f into main May 3, 2026
19 of 22 checks passed
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