fix(ci): thank first contributors on push, not on the fork's PR event

.github/workflows/first-contribution.yml

@@ -1,48 +1,77 @@
 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
           fi
 
           body="$RUNNER_TEMP/thanks.md"
           cat > "$body" <<EOF
-          Merged — thanks @$AUTHOR, that's your first one here.
+          Merged — thanks @$author, that's your first one here.
 
           If the project turned out to be useful to you, a ⭐ genuinely helps: stacktale is
           new, and stars are most of what decides whether anyone else finds it.
@@ -51,9 +80,8 @@ jobs:
           list is kept honest, and each one names the files to touch and how to verify.
           EOF
 
-          # An org policy can force workflow tokens to read-only, which makes this POST 403.
-          # Don't fail the run over a thank-you note — a red X on every merged PR is worse
-          # than a missing comment. Switch it on at Settings > 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