develop -> master

.github/workflows/release.yml

@@ -4,30 +4,54 @@ name: Release
 #
 # DESIGN: the annotated git tag `vX.Y.Z` is the SOLE source of truth. This repo
 # has NO Cargo.toml and NO VERSION file, so there is nothing in the tree to
-# rewrite. On every push/merge to `master` this workflow:
+# rewrite. When a `develop` -> `master` release PR MERGES (or on a manual
+# `workflow_dispatch`) this workflow:
 #   1. reads the highest existing vX.Y.Z git tag,
 #   2. bumps PATCH by default, or MINOR/MAJOR when the merged PR carried a
 #      `release:minor` / `release:major` label,
 #   3. creates + pushes the matching annotated vX.Y.Z tag on the merge commit
-#      (NO commit is written back to the branch — only a tag), and
+#      (NO commit is written back to the branch — only a tag),
 #   4. publishes a GitHub Release with auto-generated notes, best-effort
 #      appended with the current decomp progress (matched-function /
-#      _rosetta file counts from tools/progress.py).
+#      _rosetta file counts from tools/progress.py), and
+#   5. announces the release to Discord (see docs/RELEASING.md).
 #
-# Why NO PAT: unlike the sibling Garlemald-Server/Client release workflows,
-# `master` here is NOT branch-protected and this workflow pushes ONLY a tag
-# (never a commit back to the branch). The default GITHUB_TOKEN (github.token)
-# can both push a tag to an unprotected branch and create a Release, so no
-# fine-grained RELEASE_PAT is required. The checkout uses the default token.
+# Why `pull_request_target` and NOT `push: branches: [master]`: a release PR is
+# merged from `develop`, which is saturated with `[skip ci]` commits (the
+# reconcile bot). A SQUASH merge folds all of those commit messages — `[skip ci]`
+# included — into the squash commit's body, and GitHub then SKIPS every workflow
+# for that push (it scans the HEAD commit message for skip markers). A
+# `push: master` trigger therefore silently never fires for a squash-merged
+# release. `pull_request_target` keys off the PR-merge EVENT, not the pushed
+# commit, so it is immune to `[skip ci]` and fires for any merge method
+# (squash / merge / rebase). `workflow_dispatch` is a manual fallback (e.g. to
+# cut a release that a skipped push missed).
 #
-# Why NO loop guard: because nothing is committed back to `master`, there is no
-# re-trigger to guard against. A tag push does NOT match an `on: push: branches`
-# trigger, so creating vX.Y.Z below cannot re-run this workflow. Consequently we
-# deliberately add NO bot-identity `if` guard and NO `[skip ci]` marker — they
-# would only be needed if we pushed a bump commit, which we never do.
+# SAFETY: despite `pull_request_target`, this workflow NEVER checks out or runs
+# PR head code — it checks out `master` and only reads git tags + the merged PR's
+# labels — so the usual `pull_request_target` code-execution risk (running
+# attacker-controlled code with secrets in scope) does not apply.
+#
+# Why NO PAT: unlike the sibling Garlemald-Server/Client release workflows, this
+# workflow pushes ONLY a tag (never a commit back to the branch). Tag pushes are
+# not gated by branch protection (only commits to the branch are), so the default
+# GITHUB_TOKEN (github.token) can push the tag AND create the Release even though
+# `master` is protected — no fine-grained RELEASE_PAT is required.
+#
+# Why NO loop guard: nothing is committed back to `master`, so there is no
+# re-trigger to guard against. The tag push creates no PR-close event and matches
+# no trigger here, so it cannot re-run this workflow. We deliberately add NO
+# bot-identity `if` guard and NO `[skip ci]` marker.
 on:
-  push:
+  # Fire when a PR INTO master MERGES (a develop -> master release). For
+  # pull_request_target, `branches` filters the BASE (target) branch, so only
+  # master-targeting PRs match; the job `if` further requires merged == true.
+  pull_request_target:
+    types: [closed]
     branches: [master]
+  # Manual fallback: cut a release on demand (e.g. recover a skipped push). Always
+  # checks out + releases master HEAD regardless of the ref it is dispatched from.
+  workflow_dispatch: {}
 
 # Serialize releases so two quick merges produce monotonic tags (the second run
 # waits for the first to push its tag, then computes the next from it).
@@ -45,13 +69,22 @@ permissions:
 
 jobs:
   release:
+    # Release only when a PR actually MERGED into master (skip closed-without-merge),
+    # or on a manual dispatch. The pull_request_target `branches` filter already
+    # restricts to master-targeting PRs; the base.ref check is defense-in-depth.
+    if: >-
+      github.event_name == 'workflow_dispatch' ||
+      (github.event.pull_request.merged == true &&
+       github.event.pull_request.base.ref == 'master')
     runs-on: ubuntu-latest
     steps:
       - name: Checkout master (full history + tags; default token can push a tag)
         uses: actions/checkout@v6
         with:
-          # master is unprotected and we only push a TAG, so the default
-          # GITHUB_TOKEN suffices — no RELEASE_PAT needed.
+          # A merged pull_request_target/closed event means master already
+          # contains the merge commit, so `ref: master` checks out the release
+          # HEAD to tag. master is protected, but we only push a TAG (not gated by
+          # branch protection), so the default GITHUB_TOKEN suffices — no RELEASE_PAT.
           token: ${{ github.token }}
           fetch-depth: 0
           ref: master
@@ -63,23 +96,28 @@ jobs:
         id: level
         env:
           GH_TOKEN: ${{ github.token }}
+          REPO: ${{ github.repository }}
         run: |
           set -euo pipefail
-          # PR(s) associated with this merge commit; labels decide minor/major.
-          # A direct push (no PR) -> empty -> patch. Manual `git tag vX.Y.0`/`vX.0.0`
-          # is also honored automatically via the highest-tag read below.
-          # An empty result is the legitimate "direct push, no PR" case (-> patch).
-          # A non-zero exit is an API/permission error: warn so a missed
-          # release:minor/major label is visible rather than silently downgraded.
-          if ! labels="$(gh api "repos/${{ github.repository }}/commits/${{ github.sha }}/pulls" --jq '[.[].labels[].name] | join(",")')"; then
-            echo "::warning::Could not query PR labels for ${{ github.sha }}; defaulting to a patch bump (a release:minor/major label, if any, was NOT applied)."
+          # Read the release PR's labels via the PR(s) associated with the master
+          # HEAD (= the just-merged commit). We query by the CHECKED-OUT HEAD sha,
+          # NOT github.sha: under pull_request_target, github.sha is the base sha
+          # from BEFORE the merge, whereas master HEAD is the merge commit the
+          # release PR is associated with (under workflow_dispatch it is simply
+          # master HEAD). An empty result is the legitimate "no associated PR" case
+          # (-> patch). A non-zero exit is an API/permission error: warn so a
+          # missed release:minor/major label is visible rather than silently
+          # downgraded.
+          sha="$(git rev-parse HEAD)"
+          if ! labels="$(gh api "repos/${REPO}/commits/${sha}/pulls" --jq '[.[].labels[].name] | join(",")')"; then
+            echo "::warning::Could not query PR labels for ${sha}; defaulting to a patch bump (a release:minor/major label, if any, was NOT applied)."
             labels=""
           fi
           echo "PR labels: ${labels:-<none>}"
-          level=patch
+          level="patch"
           case ",$labels," in
-            *,release:major,*) level=major ;;
-            *,release:minor,*) level=minor ;;
+            *,release:major,*) level="major" ;;
+            *,release:minor,*) level="minor" ;;
           esac
           echo "level=$level" >> "$GITHUB_OUTPUT"
           echo "Bump level: $level"
@@ -115,10 +153,10 @@ jobs:
           tag="${{ steps.ver.outputs.tag }}"
           git config user.name  "github-actions[bot]"
           git config user.email "41898282+github-actions[bot]@users.noreply.github.com"
-          # Tag the merge commit that triggered this run (HEAD == github.sha).
+          # Tag the checked-out master HEAD — the merge commit for this release.
           # NOTHING is written into the tree and NO commit is pushed back to
-          # master — only the tag ref. A tag push does not re-trigger this
-          # workflow (the trigger is scoped to branch pushes on master).
+          # master — only the tag ref. The tag push creates no PR-close event, so
+          # it cannot re-trigger this workflow.
           git tag -a "${tag}" -m "${tag}"
           git push origin "refs/tags/${tag}"
           echo "Pushed tag ${tag}"
@@ -166,8 +204,9 @@ jobs:
       - name: Announce the release on Discord
         # Runs only after the tag + GitHub Release above succeed (a failed step
         # stops the job before this one), so a release is never announced unless
-        # it actually published. No event guard is needed: this workflow's only
-        # trigger is push to `master`, i.e. a develop -> master release merge.
+        # it actually published. No event guard is needed: both triggers (a merged
+        # develop -> master release PR, and a manual workflow_dispatch) compute and
+        # publish a NEW version, so every reached run is a genuine release.
         env:
           DISCORD_WEBHOOK: ${{ secrets.DISCORD_RELEASE_WEBHOOK_URL }}
           PROJECT: Meteor Decomp

docs/RELEASING.md

@@ -13,13 +13,14 @@ branch: it only creates and pushes a tag (and publishes a GitHub Release).
   orchestration included) or via PR, whatever fits the moment. **Nothing merged
   into `develop` produces a release or a tag.**
 - **`master`** is the protected **release** branch. A release is cut by opening a
-  PR from `develop` into `master`; when it merges, the push to `master` triggers
-  `release.yml` (tag + GitHub Release). `master` requires a pull request (no
-  direct pushes) but **no approving review**, so you can self-merge your own
-  `develop` → `master` release PR.
+  PR from `develop` into `master`; when it **merges**, `release.yml` fires (tag +
+  GitHub Release + Discord). `master` requires a pull request (no direct pushes)
+  but **no approving review**, so you can self-merge your own `develop` → `master`
+  release PR. **Any** merge method works — squash, merge commit, or rebase (see
+  [Why `pull_request_target`, not `push`](#why-pull_request_target-not-push)).
 
 Flow: work on `develop` → release PR `develop` → `master` → merge → automatic
-tag + GitHub Release.
+tag + GitHub Release + Discord announcement.
 
 > Branch protection on `master` does **not** block the release automation: the
 > workflow pushes only a **tag** (`refs/tags/vX.Y.Z`), and tag pushes are not
@@ -29,8 +30,9 @@ tag + GitHub Release.
 
 ## How it works
 
-`.github/workflows/release.yml` runs on every push/merge to **`master`** (i.e.
-when a `develop` → `master` release PR merges) and:
+`.github/workflows/release.yml` runs when a `develop` → `master` release PR
+**merges** (a `pull_request_target` `closed` + merged event), or on a manual
+`workflow_dispatch`, and:
 
 1. reads the highest existing `vX.Y.Z` tag,
 2. picks a bump level (see below),
@@ -48,13 +50,14 @@ After a release, `git describe --tags` on `master` reports the new `X.Y.Z`.
 
 | Bump      | How to trigger                                                                 |
 |-----------|--------------------------------------------------------------------------------|
-| **Patch** | Default. Any merge/push to `master` with no release label → `Z` increments.     |
+| **Patch** | Default. A merged `develop` → `master` release PR (or manual dispatch) with no release label → `Z` increments. |
 | **Minor** | Add the **`release:minor`** label to the PR before merging → `Y+1`, `Z=0`.      |
 | **Major** | Add the **`release:major`** label to the PR before merging → `X+1`, `Y=0`, `Z=0`. |
 
 The label is read from the PR associated with the merge commit; if both labels
-are present, **`release:major` wins**. A direct push with no associated PR is
-treated as a patch bump.
+are present, **`release:major` wins**. A run with no associated PR (a manual
+`workflow_dispatch`, or a merge GitHub doesn't link to a PR) is treated as a
+patch bump.
 
 The `release:minor` and `release:major` labels must exist in the repo.
 
@@ -70,30 +73,63 @@ git tag -a v0.2.0 -m v0.2.0 && git push origin v0.2.0
 The automation then continues patch-incrementing from there (`v0.2.1`, …). This
 is handy for the first minor/major when no PR is involved.
 
+## Why `pull_request_target`, not `push`
+
+The release PR merges `develop` into `master`, and `develop` is **saturated with
+`[skip ci]` commits** (the reconcile bot stamps every progress-doc commit with
+`[skip ci]`). A **squash** merge folds *all* the squashed commit messages —
+`[skip ci]` markers included — into the squash commit's body. GitHub then scans
+the merged commit's message, sees `[skip ci]`, and **skips every workflow for
+that push** (including `release.yml`). With a `push: branches: [master]` trigger,
+a squash-merged release therefore **silently never releases** — no tag, no
+GitHub Release, no Discord ping. (This is exactly what happened to the first
+release PR after the Discord step was added.)
+
+`pull_request_target` keys off the **PR-merge event**, not the pushed commit, so
+it is **immune to `[skip ci]`** and fires regardless of merge method (squash,
+merge commit, or rebase). The job is gated on `pull_request.merged == true` (so a
+PR closed *without* merging never releases) and `base.ref == 'master'`.
+
+> **Safety:** despite the `pull_request_target` event, this workflow never checks
+> out or runs PR head code — it checks out `master` and reads only git tags and
+> the merged PR's labels. The usual `pull_request_target` risk (running
+> attacker-controlled code with secrets in scope) does not apply.
+
+## Manual release (`workflow_dispatch`)
+
+`release.yml` also exposes a manual **Run workflow** button (`workflow_dispatch`,
+no inputs). Use it to cut a release on demand — e.g. to recover a push that was
+skipped before this trigger fix landed, or to force a patch bump. It always
+checks out and releases **`master` HEAD**, computing the next version from the
+highest tag exactly like an automatic run.
+
+```sh
+gh workflow run release.yml --repo swstegall/meteor-decomp
+```
+
 ## Why no PAT
 
-Unlike the sibling `garlemald-server` / `garlemald-client` release workflows,
-`master` here is **not branch-protected**, and this workflow pushes **only a
-tag** — it never commits anything back to the branch. The default
-`GITHUB_TOKEN` (`github.token`) can:
+This workflow pushes **only a tag** — it never commits anything back to the
+branch. Tag pushes are **not** gated by branch-protection rules (only commits to
+the branch are), so even though `master` is protected, the default `GITHUB_TOKEN`
+(`github.token`) can:
 
-- push a tag to an unprotected branch, and
-- create a GitHub Release.
+- push the `vX.Y.Z` tag, and
+- create the GitHub Release.
 
-So **no fine-grained `RELEASE_PAT` is required**. The checkout uses the default
-token. (If `master` were ever branch-protected, note that tag pushes are not
-gated by branch protection rules, so even then the default token would still
-suffice for the tag — protection only matters for commits to the branch, which
-this workflow never makes.)
+So **no fine-grained `RELEASE_PAT` is required** (unlike the sibling
+`garlemald-server` / `garlemald-client` release workflows, which commit a version
+bump back to a protected branch and therefore do need a PAT). The checkout uses
+the default token.
 
 ## Why no loop guard
 
 Because nothing is committed back to `master`, there is no re-trigger to guard
 against:
 
-- The workflow triggers on `push: branches: [master]`.
-- Pushing a **tag** does **not** match that trigger, so creating `vX.Y.Z`
-  cannot re-run this workflow.
+- The workflow triggers on a PR **merge** into `master` (and manual dispatch).
+- Pushing a **tag** creates no PR-close event and matches no trigger here, so
+  creating `vX.Y.Z` cannot re-run this workflow.
 
 So there is deliberately **no bot-identity `if` guard** and **no `[skip ci]`
 marker** — both are only needed when a workflow pushes a *commit* back to the
@@ -118,8 +154,9 @@ failure (PyYAML missing, network blip, `progress.py` change, no YAML present)
 After the tag and GitHub Release publish, `release.yml`'s final step POSTs a
 Discord embed announcing the new release to the release-announcements channel
 (mirrors the sibling `garlemald-server` / `garlemald-client` release announcers).
-Because the only trigger is a push to `master` (a `develop` → `master` release
-merge), every successful release is announced — there is no separate event guard.
+Because every reached run is a genuine release — a merged `develop` → `master`
+release PR, or a manual `workflow_dispatch`, both of which compute and publish a
+new version — each successful release is announced; there is no separate event guard.
 
 The embed reads:
 
@@ -154,6 +191,6 @@ once by a maintainer as a seed step **outside** this workflow:
 git tag -a v0.1.0 -m v0.1.0 <master-HEAD-sha> && git push origin v0.1.0
 ```
 
-The first merge/push to `master` after the release automation lands bumps it to
-`v0.1.1` (or the labeled minor/major). The workflow never creates `v0.1.0`
-itself.
+The first `develop` → `master` release PR merged after the release automation
+lands bumps it to `v0.1.1` (or the labeled minor/major). The workflow never
+creates `v0.1.0` itself.