diff --git a/.github/workflows/first-contribution.yml b/.github/workflows/first-contribution.yml index 2fbe853..ce35c3e 100644 --- a/.github/workflows/first-contribution.yml +++ b/.github/workflows/first-contribution.yml @@ -1,40 +1,69 @@ name: First contribution -# Thanks someone the first time a PR of theirs is merged. On merge rather than on open: -# at that point they've actually given something, so it reads as thanks instead of a pitch. +# Thanks someone the first time a PR of theirs is merged. On merge rather than on open: at +# that point they've actually given something, so it reads as thanks instead of a pitch. # -# Note on the check: author_association is NOT usable here. Merging a PR promotes its author -# from FIRST_TIME_CONTRIBUTOR to CONTRIBUTOR, and the payload delivered with the `closed` -# event already carries the new value — so a condition on it never matches. Counting the -# author's merged PRs is the signal that survives that. +# On `push` to main rather than `pull_request_target: closed`, which is the obvious trigger +# and does not work. A workflow run triggered by a fork's pull request gets a read-only +# GITHUB_TOKEN regardless of the repository's Workflow permissions setting, so the POST came +# back 403 on every real first contribution — silently, because the run itself was green. +# Granting it means enabling "send write tokens to workflows from fork pull requests", which +# hands a write token to every fork-triggered run in the repository. That is a bad trade for +# a thank-you note. A push to main is not fork-triggered, so its token honours the +# permissions block below. # -# pull_request_target is required to comment on a fork's PR — it runs with the base repo's -# permissions. It therefore MUST NOT check out or execute the PR's code; this job only -# posts a comment, and has no checkout step for that reason. +# Note on the check: author_association is NOT usable here either. Merging a PR promotes its +# author from FIRST_TIME_CONTRIBUTOR to CONTRIBUTOR before the event is delivered, so a +# condition on it never matches. Counting the author's merged PRs survives that. on: - pull_request_target: - types: [closed] + push: + branches: [main] + workflow_dispatch: + inputs: + pr: + description: "PR number to thank for (testing; skips the push-derived lookup)" + required: true permissions: + contents: read pull-requests: read issues: write jobs: thanks: - if: github.event.pull_request.merged == true && github.event.pull_request.user.type != 'Bot' runs-on: ubuntu-latest steps: - env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} REPO: ${{ github.repository }} - PR: ${{ github.event.pull_request.number }} - AUTHOR: ${{ github.event.pull_request.user.login }} + SHA: ${{ github.sha }} + MANUAL_PR: ${{ inputs.pr }} run: | set -euo pipefail + if [ -n "${MANUAL_PR:-}" ]; then + pr="$MANUAL_PR" + else + # Whatever PR this commit came from, however it was merged. Reading the API + # rather than parsing the commit subject: squash writes "(#12)", a merge commit + # writes "Merge pull request #12", and a rebase merge writes neither. + pr=$(gh api "repos/$REPO/commits/$SHA/pulls" --jq '.[0].number // empty') + fi + if [ -z "$pr" ]; then + echo "No pull request behind this commit — direct push." + exit 0 + fi + + author=$(gh api "repos/$REPO/pulls/$pr" --jq '.user.login') + type=$(gh api "repos/$REPO/pulls/$pr" --jq '.user.type') + if [ "$type" = "Bot" ]; then + echo "$author is a bot." + exit 0 + fi + merged=$(gh api --paginate "repos/$REPO/pulls?state=closed&per_page=100" \ - --jq ".[] | select(.user.login==\"$AUTHOR\" and .merged_at != null) | .number" | wc -l) - echo "$AUTHOR has $merged merged PR(s) here." + --jq ".[] | select(.user.login==\"$author\" and .merged_at != null) | .number" | wc -l) + echo "$author has $merged merged PR(s) here." if [ "$merged" -ne 1 ]; then echo "Not their first — nothing to say." exit 0 @@ -42,7 +71,7 @@ jobs: body="$RUNNER_TEMP/thanks.md" cat > "$body" < Actions > Workflow permissions. - if ! gh api --method POST "repos/$REPO/issues/$PR/comments" -F body=@"$body"; then - echo "::warning::could not comment — the workflow token is read-only for this org" + # Still tolerated rather than fatal: a red X on main over a thank-you note is worse + # than a missing note. The ::warning:: is what makes it visible instead of silent. + if ! gh api --method POST "repos/$REPO/issues/$pr/comments" -F body=@"$body"; then + echo "::warning::could not comment — check Settings > Actions > Workflow permissions" fi