Repository navigation
Run PR review only for same-repo PRs - #82
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| 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' }} |
There was a problem hiding this comment.
🟡 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.
General PR Review: Run PR review only for same-repo PRsBlocking Issues: 0 | Suggestions: 0 | Threads Resolved: 0 Review SummaryThis PR switches both review workflows from Security IssuesNone found. Correctness IssuesNone found. SuggestionsNone. |
| 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' }} |
There was a problem hiding this comment.
🟡 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.
General PR Review: Run PR review only for same-repo PRsBlocking Issues: 0 | Suggestions: 1 | Threads Resolved: 0 Review SummaryThis PR switches both review workflows from Security IssuesNone found. Correctness IssuesNone found. Suggestions
Prompt for AI agents |
731d75f to
43ca2fb
Compare
## Why Pull requests to this repository get no Claude review. The organization ruleset that requires `general-pr-review.yaml` on managed repositories covers this repository too, and the injected run does start here, but the job's `if:` skipped it: the condition was `github.event_name != 'pull_request' || github.repository != 'ConductorOne/github-workflows'`, and the injected run is a `pull_request` event in this repository. A skipped required job counts as passing, so every PR here merged with the review check reporting "skipped" (see #140). ## What this changes Deletes that `if:` line. The event half of the condition only existed to exempt `workflow_call` callers from the self-skip, so once the repository check goes there is nothing left for it to do. Nothing else changes: fork PRs are still skipped by the same-repo guard on each step, and the connector workflow `pr-review.yaml` keeps its own self-skip, so this repository gets exactly one review per PR instead of two racing on the same summary comment. ## Where the self-skip came from Nobody recorded the reason, so here is the reconstruction from git and review history. 1. **#50 (February), the original reviewer.** The workflow was `pull_request_target`-triggered and lived in this repository, so it fired on this repository's own PRs. Its prompt opened with "You are reviewing a pull request for a ConductorOne connector repository", so the one repository that is not a connector excluded itself. No comment or PR text explains it; that is the inferred origin. 2. **#76 (May 13), the general profile.** `general-pr-review.yaml` was created by copying the connector workflow and the clause came along verbatim, even though the connector-prompt reason no longer applied to the general profile. 3. **#82 (May 21 to 28), the fork fix.** The first commit replaced the self-skip on both files with a fork-skip, so this repository would have been reviewed. The bot reviewer then flagged that both workflows now fire on this repository's PRs with the same `### General PR Review:` heading and race to update one comment. The response came in two steps: "Guard PR review against stale heads" put the self-skip back on the connector workflow only, narrowed to native `pull_request` events so reusable-workflow callers still run, and the reviewer's next summary records the intent that only the general workflow runs for this repository. Then "Skip general PR review self-runs" added the same clause to the general workflow, with no body and no thread. The managed-repository ruleset had been created a week earlier and already injected the general workflow here, so the likely trigger was a second duplicate: the injected run plus the native run of the same file. The clause chosen skips both, because the injected run is also a `pull_request` event in this repository. So the clause's surviving purpose is duplicate prevention, and the last step overshot: it removed this repository's review entirely rather than only the redundant native run. On #140 only one general run existed, the injected one, so the duplicate from May does not reproduce today. ## Rollout The injected run always executes the workflow from `main`, so this PR is still reviewed by the old condition and reports skipped. The first review lands on the next PR after this merges. The injected run checks out the workflow repository through contexts that the required-workflow machinery fills in; on a managed repository they resolve to this repository at `main`, and the same is expected here, but the self-repository case cannot be exercised before merge. If it did fail, the next PR's required check fails loudly and the revert is this one line. ## Validation - `actionlint` on the changed file: the only findings are the pre-existing `job.workflow_repository` / `job.workflow_sha` expressions, unchanged by this PR. - `git diff --check` clean. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
2 participants
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
Validation