Skip to content

Run PR review only for same-repo PRs#82

Open
gontzess wants to merge 2 commits into
mainfrom
gontzess/same-repo-pr-review
Open

Run PR review only for same-repo PRs#82
gontzess wants to merge 2 commits into
mainfrom
gontzess/same-repo-pr-review

Conversation

@gontzess
Copy link
Copy Markdown
Contributor

@gontzess gontzess commented May 21, 2026

Why

The PR review workflow needs to restore high-quality source inspection without running secret-backed Claude review on public fork PRs. Same-repo PRs are the intended automatic review scope; fork PRs should skip clearly instead of entering a privileged review path.

What this changes

  • Switches connector and general PR review workflows from pull_request_target to pull_request.
  • Skips fork PRs with an explicit successful message.
  • Checks out the exact PR head SHA for same-repo PRs so Claude can inspect local source.
  • Loads the review action from the workflow commit instead of @main.
  • Pins the Claude action and artifact upload action by SHA.
  • Filters human comment context to owner, member, and collaborator comments.
  • Verifies outdated review threads are bot-authored before resolving them.

Validation

  • yq parsed both PR review workflows and the composite action.
  • Python scripts compiled with py_compile.
  • git diff --check passed.

github-actions[bot]
github-actions Bot reviewed May 21, 2026
View reviewed changes
Comment thread .github/workflows/pr-review.yaml Outdated
github_token: ${{ secrets.GITHUB_TOKEN }}
pr_number: ${{ github.event.pull_request.number }}
review_prompt: ${{ inputs.review_prompt || 'connector' }}
review_prompt: ${{ github.repository == 'ConductorOne/github-workflows' && 'general' || inputs.review_prompt || 'connector' }}
Copy link
Copy Markdown

@github-actions github-actions Bot May 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Suggestion: With the if: github.repository != 'ConductorOne/github-workflows' guard removed from both workflows, pr-review.yaml and general-pr-review.yaml will both fire on pull_request events for ConductorOne/github-workflows. Both resolve to the general profile with the same ### General PR Review: summary heading, producing duplicate reviews with separate summary comments. Consider re-adding the guard here or on general-pr-review.yaml so only one runs on this repo.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 21, 2026 •
edited
Loading

General PR Review: Run PR review only for same-repo PRs

Blocking Issues: 0 | Suggestions: 0 | Threads Resolved: 0
Review mode: full
View review run

Review Summary

This PR switches both review workflows from pull_request_target to pull_request, restricts review to same-repo PRs (fork PRs are skipped), checks out PR head code for local inspection, pins actions by SHA, filters PR comments to trusted author associations only, and adds SHA verification to prevent reviewing stale code. The previous finding about duplicate reviews on the github-workflows repo is addressed — pr-review.yaml now skips when the event is pull_request and the repository is ConductorOne/github-workflows, so only general-pr-review.yaml runs for this repo. The thread resolution script now verifies all comments in a thread are bot-authored before auto-resolving, preventing resolution of threads with human replies.

Security Issues

None found.

Correctness Issues

None found.

Suggestions

None.

github-actions[bot]
github-actions Bot reviewed May 21, 2026
View reviewed changes
Copy link
Copy Markdown

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No blocking issues found.

github-actions[bot]
github-actions Bot reviewed May 21, 2026
View reviewed changes
Comment thread .github/workflows/pr-review.yaml Outdated
github_token: ${{ secrets.GITHUB_TOKEN }}
pr_number: ${{ github.event.pull_request.number }}
review_prompt: ${{ inputs.review_prompt || 'connector' }}
review_prompt: ${{ github.repository == 'ConductorOne/github-workflows' && 'general' || inputs.review_prompt || 'connector' }}
Copy link
Copy Markdown

@github-actions github-actions Bot May 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Suggestion: With the if: github.repository != 'ConductorOne/github-workflows' guard removed from both workflows, PRs to the github-workflows repo will now trigger both pr-review.yaml and general-pr-review.yaml. Both resolve to review_prompt: general and produce ### General PR Review: summaries, so they'll race to create/update the same comment. Consider gating one of the two workflows to skip on this repo, or giving them distinct summary headings.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 21, 2026

General PR Review: Run PR review only for same-repo PRs

Blocking Issues: 0 | Suggestions: 1 | Threads Resolved: 0
Review mode: full
View review run

Review Summary

This PR switches both review workflows from pull_request_target to pull_request, skips fork PRs, checks out PR head for local source inspection, pins actions by SHA, filters PR comments to trusted author associations, and adds a bot-authorship check before resolving outdated threads. The changes are well-structured security improvements. One suggestion was raised about duplicate workflow triggers on the github-workflows repo itself.

Security Issues

None found.

Correctness Issues

None found.

Suggestions

  • 🟡 Both pr-review.yaml and general-pr-review.yaml will trigger on PRs to this repo with the same ### General PR Review: heading, causing duplicate reviews — .github/workflows/pr-review.yaml:48
Prompt for AI agents 
Verify each finding against the current code and only fix it if needed.

## Suggestions

In `.github/workflows/pr-review.yaml`:
- Around line 48: Both `pr-review.yaml` and `general-pr-review.yaml` now have `on: pull_request` triggers and neither skips the `ConductorOne/github-workflows` repo (the old `if: github.repository != 'ConductorOne/github-workflows'` guard was removed from both). Both resolve to `review_prompt: general` for this repo, producing the same `### General PR Review:` summary heading. This means PRs to the github-workflows repo will trigger duplicate Claude reviews that race to create/update the same summary comment. Fix by either adding an `if: github.repository != 'ConductorOne/github-workflows'` condition back to one of the two workflows, or by giving them distinct summary headings so they don't conflict.

github-actions[bot]
github-actions Bot reviewed May 21, 2026
View reviewed changes
Copy link
Copy Markdown

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No blocking issues found.

github-actions[bot]
github-actions Bot reviewed May 21, 2026
View reviewed changes
Copy link
Copy Markdown

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No blocking issues found.

@gontzess gontzess marked this pull request as ready for review May 22, 2026 00:33
github-actions[bot]
github-actions Bot reviewed May 22, 2026
View reviewed changes
Copy link
Copy Markdown

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No blocking issues found.

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

Reviewers

@github-actions github-actions[bot] github-actions[bot] left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@gontzess