-
Notifications
You must be signed in to change notification settings - Fork 3
feat: trigger skill review separately #191
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 |
|---|---|---|
|
|
@@ -35,4 +35,40 @@ jobs: | |
| uses: anthropics/claude-code-action@v1 | ||
| with: | ||
| anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }} | ||
| custom_instructions: "Skills are available in .claude/skills/. Users can invoke them with /review or /commit in comments." | ||
|
|
||
| claude-review: | ||
| if: | | ||
| (github.event_name == 'issue_comment' && contains(github.event.comment.body, '@review')) || | ||
| (github.event_name == 'pull_request_review_comment' && contains(github.event.comment.body, '@review')) | ||
| runs-on: ubuntu-latest | ||
| permissions: | ||
| contents: read | ||
| pull-requests: write | ||
| issues: write | ||
| id-token: write | ||
| actions: read | ||
| steps: | ||
| - name: Checkout repository | ||
| uses: actions/checkout@v6 | ||
| with: | ||
| fetch-depth: 1 | ||
|
|
||
| - name: Load review skill | ||
| id: skill | ||
| run: | | ||
| if [ -f ".claude/skills/review.md" ]; then | ||
| SKILL_CONTENT=$(cat .claude/skills/review.md) | ||
| echo "skill_content<<EOF" >> $GITHUB_OUTPUT | ||
| echo "$SKILL_CONTENT" >> $GITHUB_OUTPUT | ||
| echo "EOF" >> $GITHUB_OUTPUT | ||
| else | ||
| echo "skill_content=Perform a thorough code review." >> $GITHUB_OUTPUT | ||
| fi | ||
|
|
||
| - name: Run Claude Review | ||
| id: claude | ||
| uses: anthropics/claude-code-action@v1 | ||
| with: | ||
| anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }} | ||
| prompt: | | ||
| ${{ steps.skill.outputs.skill_content }} | ||
|
Comment on lines
+68
to
+74
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 The Extended reasoning...What the bug isThe How it manifestsIn Since the Why existing code doesn't prevent itCompare the two jobs: the Additionally, the Step-by-step proof
How to fix itReplace the |
||
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
claude-reviewtrigger conditions have a few gaps: (1)issue_commentfires for both issues and PRs, so@reviewon a regular issue triggers a code review job with no diff to review — adding&& github.event.issue.pull_requestwould fix this; (2) a comment containing both@claudeand@reviewtriggers both jobs simultaneously, producing duplicate work; (3) thepull_request_review(submitted) event is handled by theclaudejob but notclaude-review, so@reviewin a review summary is silently ignored.Extended reasoning...
The issue_comment event fires for regular issues too
In GitHub Actions, the
issue_commentevent fires for comments on both issues and pull requests (since PRs are a type of issue). Theclaude-reviewjob condition on line 41 checks:This means if someone writes
@reviewon a regular issue (not a PR), the job will trigger and attempt to run a code review viaclaude-code-actionwith thereview.mdprompt. Since there is no diff or changed files on a regular issue, the action will either fail or produce nonsensical output, wasting CI resources.The fix is straightforward: add
&& github.event.issue.pull_requestto theissue_commentcondition to ensure it only fires on PR comments. The originalclaudejob can get away without this check because it is a general-purpose assistant that can handle issue interactions, butclaude-reviewis specifically a code review tool that requires a PR context.Double triggering with @claude
The
claudejob triggers oncontains(comment.body, '@claude')andclaude-reviewtriggers oncontains(comment.body, '@review'). These conditions are non-exclusive. A comment like "@claude please @review this PR" would trigger both jobs simultaneously, producing duplicate review comments. One could argue this is intentional (the user asked for both), but it is still worth noting since the two jobs would produce overlapping code review output — theclaudejob would see@claudeand respond generically, whileclaude-reviewwould run the dedicated review skill, leading to redundant and potentially conflicting feedback.Missing pull_request_review event
The workflow-level
on:block includespull_request_review: types: [submitted](line 10-11), and theclaudejob handles this event type (line 18). However, theclaude-reviewjob only handlesissue_commentandpull_request_review_comment— it does not check for@reviewin the overall review submission body. This is a minor inconsistency since requesting an automated review from within your own review submission is an uncommon workflow, but it breaks the symmetry with theclaudejob pattern.Concrete example for the issue_comment bug
Step-by-step proof for the most actionable issue:
issue_commentevent fires withgithub.event_name == 'issue_comment'contains(github.event.comment.body, '@review')evaluates to trueclaude-reviewjob starts, checks out the repo, loadsreview.md, and invokesclaude-code-actionwith a review prompt — but there is no PR diff to reviewSuggested fix
Change line 41 from:
(github.event_name == 'issue_comment' && contains(github.event.comment.body, '@review'))to:
(github.event_name == 'issue_comment' && github.event.issue.pull_request && contains(github.event.comment.body, '@review'))Optionally, also add a
pull_request_reviewcondition and make the@reviewcondition exclude@claude(or vice versa) to address the other two issues.