diff --git a/.github/workflows/claude-code-review.yml b/.github/workflows/claude-code-review.yml index ae19f78b..343d3ce1 100644 --- a/.github/workflows/claude-code-review.yml +++ b/.github/workflows/claude-code-review.yml @@ -17,10 +17,21 @@ name: Claude Code Review on: pull_request: - # `labeled` is the manual re-run: a PR opened before this workflow existed - # has no run to re-run from the Actions tab, and reopening fires `reopened` - # rather than `opened`. Applying the label needs write access, so the same - # label doubles as the approval gate in claude-review-fork.yml. + # `labeled` is the manual re-run: reopening a PR fires `reopened`, not + # `opened`, and a PR that predates this file has no run to restart from the + # Actions tab. Applying the label needs write access, so the same label + # doubles as the approval gate in claude-review-fork.yml. + # + # It does not reach the PRs that need it most, and the reason is worth + # knowing before you conclude the label is broken. A `pull_request` workflow + # is read from the PR's merge ref, which GitHub recomputes on a push and not + # otherwise. A PR whose last push predates this file therefore has a merge + # ref that does not contain it, and no label will summon a workflow that, + # from that ref's point of view, does not exist. Push to the branch and the + # merge ref catches up -- though by then the push has triggered + # `synchronize` anyway, which is why the empty-commit trick works and the + # label appears not to. `claude-review-fork.yml` is exempt: a + # `pull_request_target` workflow is read from the base branch. types: [opened, synchronize, labeled] # Same reasoning as ci.yml: a superseded review is dead weight, and unlike a diff --git a/.github/workflows/claude-review-fork.yml b/.github/workflows/claude-review-fork.yml index 2eef1b54..a1fbfad0 100644 --- a/.github/workflows/claude-review-fork.yml +++ b/.github/workflows/claude-review-fork.yml @@ -69,13 +69,22 @@ jobs: name: claude review (fork) runs-on: ubuntu-latest timeout-minutes: 30 - # `labeled` carries the label that was just applied; `synchronize` carries - # none, so it has to look at what the PR is already wearing. The two need - # separate clauses: testing only the label list would spend a review every - # time any other label -- `bug`, `blocked` -- landed on a subscribed PR. + # First clause: forks only. Everything below trades review quality for + # safety against code we do not control -- no plugin, no whole-repo context, + # read-only tools -- and a branch in this repository has earned none of that. + # `pull_request_target` does not care where the head is, so if this job does + # not say so, labelling one of our own PRs lands it here and it silently + # gets the weaker review instead of the one in claude-code-review.yml. + # + # Then: `labeled` carries the label that was just applied; `synchronize` + # carries none, so it has to look at what the PR is already wearing. Those + # two need separate clauses, because testing only the label list would spend + # a review every time an unrelated label -- `bug`, `blocked` -- landed on a + # subscribed PR. if: >- - (github.event.action == 'labeled' && github.event.label.name == 'claude-review') || - (github.event.action == 'synchronize' && contains(github.event.pull_request.labels.*.name, 'claude-review')) + github.event.pull_request.head.repo.full_name != github.repository && + ((github.event.action == 'labeled' && github.event.label.name == 'claude-review') || + (github.event.action == 'synchronize' && contains(github.event.pull_request.labels.*.name, 'claude-review'))) permissions: contents: read pull-requests: write # the last step posts the review comment