Skip to content

ci: review this repository's own pull requests - #141

Merged
jugonzalez12 merged 1 commit into
mainfrom
jugonzalez/general-review-self-runs
Oct 2, 2026
Merged

jugonzalez12 merged 1 commit into
mainfrom
jugonzalez/general-review-self-runs

Conversation

@jugonzalez12

Copy link
Copy Markdown
Contributor

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. feat: add Claude-powered PR review workflow #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. Add general PR review workflow #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. Run PR review only for same-repo PRs #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

general-pr-review.yaml skipped its job whenever the event was pull_request
and the repository was ConductorOne/github-workflows. The organization
ruleset that requires this workflow covers this repository too, and its
injected run is also a pull_request event here, so the clause skipped that
run as well: pull requests to this repository received no review, and the
skipped job satisfied the ruleset.

The clause was duplicate prevention from #82, where both review workflows
briefly ran on this repository and raced on one summary comment. The
connector workflow keeps its own self-skip, so removing this one yields a
single review per pull request.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@jugonzalez12
jugonzalez12 requested a review from gontzess October 2, 2026 21:08
@jugonzalez12
jugonzalez12 merged commit cafb63b into main Oct 2, 2026
2 checks passed
@jugonzalez12
jugonzalez12 deleted the jugonzalez/general-review-self-runs branch October 2, 2026 21:23
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.

2 participants