fix: address review findings on the release workflow

.github/workflows/release-project-in-dir.yml

@@ -17,6 +17,22 @@ env:
   # set the target pom to use the input directory as root
   MAVEN_ARGS: -V -ntp -e -f ${{ inputs.project_dir }}/pom.xml
   ROOT_POM: ${{ inputs.project_dir }}/pom.xml
+  # Prints the version of the pom given as argument, falling back to the
+  # parent's version. Used instead of `help:evaluate`, whose banner and log
+  # output would have to be filtered off stdout first. Kept inline rather than
+  # as a script in the repo because the jobs check out the release branch, and
+  # older maintenance branches would not have such a script.
+  POM_VERSION_PY: |
+    import sys
+    import xml.etree.ElementTree as ET
+    NS = "{http://maven.apache.org/POM/4.0.0}"
+    root = ET.parse(sys.argv[1]).getroot()
+    version = root.findtext(NS + "version")
+    if version is None:
+        version = root.findtext(NS + "parent/" + NS + "version")
+    if version is None:
+        raise SystemExit("no version and no parent version in " + sys.argv[1])
+    print(version.strip())
 
 jobs:
   publish:
@@ -29,11 +45,43 @@ jobs:
         uses: actions/checkout@v7
         with:
           ref: "${{ inputs.version_branch }}"
+          # Full history and tags are needed to check where the release tag
+          # currently points relative to the branch.
+          fetch-depth: 0
+          fetch-tags: true
 
       - name: Resolve checked-out commit
         id: resolve-sha
         run: echo "commit=$(git rev-parse HEAD)" >> "$GITHUB_OUTPUT"
 
+      - name: Check the release tag can be moved onto this branch
+        env:
+          RELEASE_TAG: ${{ inputs.release_tag }}
+          TARGET_BRANCH: ${{ inputs.version_branch }}
+        run: |
+          set -euo pipefail
+
+          # finalize-release moves the tag onto a commit built on top of this
+          # one, which is only legitimate if the tag already points into this
+          # branch's history. Checking it here rather than after the deploy
+          # means a release cut from an unrelated commit fails while it can
+          # still be retried - once artifacts are in Maven Central they are
+          # immutable.
+          #
+          # Peel the tag in a separate step from testing its existence: a tag
+          # that resolves to something other than a commit must fail loudly
+          # rather than look like an absent tag and skip the check.
+          if git rev-parse -q --verify "refs/tags/${RELEASE_TAG}" >/dev/null; then
+            if ! CURRENT_TAGGED="$(git rev-parse -q --verify "refs/tags/${RELEASE_TAG}^{commit}")"; then
+              echo "Tag ${RELEASE_TAG} does not resolve to a commit"
+              exit 1
+            fi
+            if ! git merge-base --is-ancestor "${CURRENT_TAGGED}" HEAD; then
+              echo "Tag ${RELEASE_TAG} points at ${CURRENT_TAGGED}, which is not an ancestor of ${TARGET_BRANCH}"
+              exit 1
+            fi
+          fi
+
       - name: Set up Java and Maven
         uses: actions/setup-java@v6
         with:
@@ -49,6 +97,7 @@ jobs:
       - name: Change version to release version
         env:
           RELEASE_TAG: ${{ inputs.release_tag }}
+          TARGET_BRANCH: ${{ inputs.version_branch }}
         run: |
           set -euo pipefail
           RELEASE_VERSION="${RELEASE_TAG#v}"
@@ -63,6 +112,20 @@ jobs:
             exit 1
           fi
 
+          # finalize-release derives the next development version from the
+          # released one, so releasing a version older than the branch already
+          # develops would set the branch back onto versions that are already
+          # released. That means the wrong branch was selected for this tag.
+          # Checked here because the deploy that follows cannot be undone.
+          DEVELOPMENT_VERSION="$(python3 -c "${POM_VERSION_PY}" "${ROOT_POM}")"
+          DEVELOPMENT_BASE="${DEVELOPMENT_VERSION%-SNAPSHOT}"
+          if [ "${RELEASE_VERSION}" != "${DEVELOPMENT_BASE}" ] && \
+             [ "$(printf '%s\n%s\n' "${RELEASE_VERSION}" "${DEVELOPMENT_BASE}" | sort -V | head -1)" = "${RELEASE_VERSION}" ]; then
+            echo "${TARGET_BRANCH} develops ${DEVELOPMENT_VERSION}, which is newer than ${RELEASE_VERSION}"
+            echo "Is ${RELEASE_TAG} being released from the right branch?"
+            exit 1
+          fi
+
           ./mvnw ${MAVEN_ARGS} versions:set -DnewVersion="${RELEASE_VERSION}" versions:commit -DprocessAllModules
 
       - name: Publish to Apache Maven Central
@@ -106,17 +169,16 @@ jobs:
         id: commits
         env:
           RELEASE_TAG: ${{ inputs.release_tag }}
+          TARGET_BRANCH: ${{ inputs.version_branch }}
         run: |
           set -euo pipefail
 
-          # Reads the version of the root pom directly, rather than through
-          # help:evaluate, whose banner and log output would have to be filtered
-          # out of stdout first.
           pom_version() {
-            python3 -c 'import sys, xml.etree.ElementTree as ET; ns = "{http://maven.apache.org/POM/4.0.0}"; root = ET.parse(sys.argv[1]).getroot(); version = root.findtext(ns + "version") or root.findtext(ns + "parent/" + ns + "version"); print(version.strip())' "${ROOT_POM}"
+            python3 -c "${POM_VERSION_PY}" "${ROOT_POM}"
           }
 
           RELEASE_VERSION="${RELEASE_TAG#v}"
+          DEVELOPMENT_VERSION="$(pom_version)"
 
           git config --local user.email "action@github.com"
           git config --local user.name "GitHub Action"
@@ -160,6 +222,17 @@ jobs:
               ;;
           esac
 
+          # The bump is derived from the released version, so releasing a tag
+          # older than the branch's own development version would move the
+          # branch backwards - onto versions that have already been released.
+          # That means the wrong branch was selected for the tag, so stop.
+          if [ "${NEXT_VERSION}" != "${DEVELOPMENT_VERSION}" ] && \
+             [ "$(printf '%s\n%s\n' "${NEXT_VERSION}" "${DEVELOPMENT_VERSION}" | sort -V | head -1)" = "${NEXT_VERSION}" ]; then
+            echo "Next development version ${NEXT_VERSION} would move ${TARGET_BRANCH} back from ${DEVELOPMENT_VERSION}"
+            echo "Is ${RELEASE_TAG} being released from the right branch?"
+            exit 1
+          fi
+
           if git diff --quiet; then
             echo "Branch would be left on release version ${RELEASE_VERSION}"
             exit 1
@@ -168,6 +241,7 @@ jobs:
           echo "Next development version: ${NEXT_VERSION}"
 
       - name: Move release tag onto the release commit
+        id: tag
         env:
           RELEASE_TAG: ${{ inputs.release_tag }}
           RELEASE_COMMIT: ${{ steps.commits.outputs.release_commit }}
@@ -176,30 +250,46 @@ jobs:
 
           # GitHub created the tag on whatever the branch tip was when the
           # release was published, so it is expected to move - but only forward,
-          # onto a descendant. Anything else means the release was cut from a
-          # commit this workflow did not build, and silently discarding it would
-          # lose the tagged state.
-          if git rev-parse -q --verify "refs/tags/${RELEASE_TAG}^{commit}" >/dev/null; then
+          # onto a descendant. publish already checked this against the branch;
+          # repeat it here because the release commit is what actually gets
+          # tagged.
+          #
+          # The OID recorded here is the tag ref itself, not the commit it peels
+          # to, because that is what the push has to lease against. An empty
+          # value means the tag did not exist, which --force-with-lease reads as
+          # "must still not exist".
+          if git rev-parse -q --verify "refs/tags/${RELEASE_TAG}" >/dev/null; then
+            EXPECTED_TAG_OID="$(git rev-parse "refs/tags/${RELEASE_TAG}")"
             CURRENT_TAGGED="$(git rev-parse "refs/tags/${RELEASE_TAG}^{commit}")"
             if ! git merge-base --is-ancestor "${CURRENT_TAGGED}" "${RELEASE_COMMIT}"; then
               echo "Tag ${RELEASE_TAG} points at ${CURRENT_TAGGED}, which is not an ancestor of ${RELEASE_COMMIT}"
               exit 1
             fi
+          else
+            EXPECTED_TAG_OID=""
           fi
+          echo "expected_tag_oid=${EXPECTED_TAG_OID}" >> "$GITHUB_OUTPUT"
 
           git tag -f -a "${RELEASE_TAG}" "${RELEASE_COMMIT}" -m "Release ${RELEASE_TAG}"
 
       - name: Push release commit, next development commit and tag
         env:
           RELEASE_TAG: ${{ inputs.release_tag }}
           TARGET_BRANCH: ${{ inputs.version_branch }}
+          EXPECTED_TAG_OID: ${{ steps.tag.outputs.expected_tag_oid }}
         run: |
           set -euo pipefail
 
           # One atomic push so the branch is never left holding the release
-          # commit without the SNAPSHOT commit that follows it. The branch
-          # refspec is not forced: if something landed on the branch while the
-          # release was being deployed, this fails instead of clobbering it.
-          git push --atomic origin \
+          # commit without the SNAPSHOT commit that follows it.
+          #
+          # Neither ref can be clobbered: the branch refspec is not forced, so a
+          # commit landing during the deploy fails the push, and the tag is
+          # leased against the OID observed during the ancestry check, so a tag
+          # moved by anyone else since then fails it too. --atomic means either
+          # both refs update or neither does.
+          git push --atomic \
+            --force-with-lease="refs/tags/${RELEASE_TAG}:${EXPECTED_TAG_OID}" \
+            origin \
             "HEAD:refs/heads/${TARGET_BRANCH}" \
-            "+refs/tags/${RELEASE_TAG}"
+            "refs/tags/${RELEASE_TAG}:refs/tags/${RELEASE_TAG}"

.github/workflows/release.yml

@@ -5,8 +5,10 @@ on:
   release:
     types: [ released ]
 
-# Releases push commits to the branch they are cut from, so run them one at a
-# time rather than letting two overlap on the same branch.
+# Two releases cut from the same branch must not overlap: both would deploy to
+# Maven Central, then one would lose the branch push and be left with immutable
+# artifacts and no release commit. Serializing means the worst case is instead a
+# release that does not start, which can simply be re-run.
 concurrency:
   group: ${{ github.workflow }}
   cancel-in-progress: false