Realign the release branch on its remote during checkout

[AliSoftware]

What does it do?

Part of AINFRA-2725, following the WooCommerce iOS 25.1 release incident. The same change is going out to every mobile product repo.

1. Realign the release branch on the remote (the actual fix)

.buildkite/commands/checkout-release-branch.sh fetched the release branch and checked it out, but never moved the local branch onto the fetched commit:

git fetch origin "$BRANCH_NAME"
git checkout "$BRANCH_NAME"

Buildkite cleans the working copy between jobs, but it can reuse it. A refs/heads/release/x.y left behind by an earlier job on the same agent therefore survives, and git checkout then simply switches to that stale local ref rather than to what was just fetched. Whatever runs next — the version bump, or the GitHub Release draft that finalize_release creates from HEAD — would then be based on the wrong commit.

The fix adds the missing realignment. reset --hard rather than git pull: no extra network round trip, and no merge commit if the refs diverged.

2. Reset to FETCH_HEAD rather than the remote-tracking ref

git fetch origin <branch> always writes FETCH_HEAD, but it only updates refs/remotes/origin/<branch> when the remote's fetch refspec covers that branch. With Buildkite's default +refs/heads/*:refs/remotes/origin/* the two are equivalent — but on a clone whose refspec was narrowed after origin/<branch> already existed, the fetch leaves that ref stale and reset --hard "origin/$BRANCH_NAME" lands on the old commit: the very failure this script exists to prevent, reintroduced through the back door. Resetting to FETCH_HEAD drops the refspec dependency entirely.

3. Standardize the script across all repos

Slightly tangent to the issue, but bundled deliberately: the script had drifted into five different shapes across the repos — argument required via ${1?…} or ${1:?…}, argument plus a BUILDKITE_BRANCH fallback, argument with a hand-rolled usage check, the same under a different variable name, and (in one repo) no argument at all, reading RELEASE_VERSION from the environment. Having rolled the same one-line fix out thirteen times this week, that divergence is pure friction.

All repos now share one canonical script. The resolution order is a superset of what every repo did before — argument, then RELEASE_VERSION from the environment, then the release/* branch the build runs on — so no call site needed changing. The BUILDKITE_BRANCH fallback only fires on a branch matching ^release/, and derives the branch name back from that same value, so it cannot select a branch other than the one the build was already triggered on.

It also closes two latent bugs: under bash -eu, both [[ -z "${RELEASE_VERSION}" ]] on an unset variable and a bare RELEASE_VERSION=$1 with no arguments abort with unbound variable before their intended usage message can print. And an argument that is passed but empty — what happens when a pipeline forwards an unset $RELEASE_VERSION — no longer resolves to a bare release/: it falls through to the environment variable, then to the release/* branch the build runs on, and finally to a clear error if none of those yields a version.

4. Bump release-toolkit to 14.11.2

Picks up wordpress-mobile/release-toolkit#763, the first half of AINFRA-2725: publish_github_release now publishes the most recently created GitHub Release when several share the same name, rather than whichever one the API happened to list first. Without it, a re-run of finalize_release can still leave the git tag on the wrong commit — the root cause of the 25.1 incident.

bundle update also refreshed a few unrelated transitive gems and bumped BUNDLED WITH to the current 4.0.17.

Testing instructions

No behaviour change on a fresh checkout, which is the normal case: the local branch is already at the fetched commit, so the reset is a no-op. The argument-resolution logic was exercised across all combinations (argument / empty argument / environment variable / release/* branch / trunk / feature branch / unset), and the resulting script passes shellcheck in every repo. The next release build exercising this script is the real check.

🤖 Generated with Claude Code

[claude]

Claude finished @AliSoftware's task in 37s —— View job

---

PR Review: Realign the release branch on its remote during checkout

• Read the changed script and analyze the fix
• Assess correctness, edge cases, and alternatives

---

The change is correct, minimal, and well-reasoned. The root cause analysis in the PR description matches what the script does: with a reused Buildkite working copy, a stale local refs/heads/release/x.y survives, git checkout switches to it rather than to what was just fetched, and the added git reset --hard "origin/$BRANCH_NAME" realigns it. Preferring reset --hard over git pull is the right call — no extra fetch, and no merge commit on divergence.

Correctness notes

• The reset depends on origin/$BRANCH_NAME being up to date, which the preceding git fetch origin "$BRANCH_NAME" (line 24) does handle: with the default clone refspec (+refs/heads/*:refs/remotes/origin/*), a named-branch fetch also updates the corresponding remote-tracking ref, so reset --hard "origin/$BRANCH_NAME" targets the freshly-fetched commit. Good — the two lines are consistent.
• Fresh-checkout case is a genuine no-op, as described: git checkout DWIM-creates the local branch already pointing at origin/$BRANCH_NAME, so the reset changes nothing. Low risk.
• set -eu (line 1) means a failed reset --hard aborts the job rather than silently continuing on a stale commit — which is the desired behaviour here.

Optional suggestion (not blocking)

The checkout-then-reset pair could be collapsed into a single atomic step:

git checkout -B "$BRANCH_NAME" "origin/$BRANCH_NAME"

-B creates the branch if missing or resets it to the start point if it exists, guaranteeing the local ref lands on the fetched commit in one operation and removing the transient window where HEAD sits on the stale commit. That said, the explicit two-line form with its comment is arguably clearer about why the reset is there, so keeping the current approach is perfectly reasonable — this is a style preference, not a defect.

No bugs, security, or performance concerns. LGTM. ✅
• ainfra-2725/reset-release-branch-checkout

[AliSoftware]

Re: the optional suggestion from the Claude Code review above: see my answer on Automattic/pocket-casts-android#5686 (comment)

[claude]

Claude finished @AliSoftware's task in 38s —— View job

---

PR Review: Realign the release branch on its remote during checkout

• Read the changed script and latest commit
• Verify the wording change addresses @mokagio's review feedback
• Re-assess correctness after the update

---

The latest push (17d136e) applies @mokagio's wording suggestion — the comment now reads plainly about what the reset does rather than the ambiguous "realign it on the remote":

# Buildkite can reuse a working copy where "$BRANCH_NAME" was left at an older commit by a previous job,
# so force the local branch to the fetched commit. `reset --hard` rather than
# `git pull`, to avoid merging if the two diverged.
git reset --hard "origin/$BRANCH_NAME"

This resolves the only outstanding comment. "force the local branch to the fetched commit" is unambiguous — no reader can now mistake it for an operation performed on the remote.

Correctness (unchanged, still holds)

• git fetch origin "$BRANCH_NAME" (line 24) updates the remote-tracking ref via the default clone refspec, so reset --hard "origin/$BRANCH_NAME" (line 29) targets the freshly-fetched commit — the two lines are consistent.
• Fresh-checkout case remains a genuine no-op, as the PR description states.
• set -eu (line 1) ensures a failed reset aborts the job rather than continuing on a stale commit.

No bugs, security, or performance concerns. The comment-only change is purely cosmetic and behaviour-preserving.

Reminder (from the PR description): hold merge until release-toolkit#763 lands and the gem/Gemfile.lock bump can be included here, so both halves of AINFRA-2725 land together.

LGTM ✅
• ainfra-2725/reset-release-branch-checkout

[claude]

Claude finished @AliSoftware's task in 1m 19s —— View job

---

PR Review: Realign the release branch on its remote during checkout

• Read the two new commits since the last review (01700b3, 27180fb)
• Verify the FETCH_HEAD change is correct
• Re-assess the standardization refactor

---

Two changes landed since my last review, both improvements.

1. git reset --hard FETCH_HEAD instead of origin/$BRANCH_NAME (01700b3)

This is a genuine robustness fix, not just a rename. The reasoning in the commit is accurate:

git fetch origin "$BRANCH_NAME"
git checkout "$BRANCH_NAME"
git reset --hard FETCH_HEAD

• git fetch origin <branch> updates the remote-tracking ref origin/<branch> only opportunistically — it happens under the default clone refspec (+refs/heads/*:refs/remotes/origin/*), but a single-branch clone, a narrowed remote.origin.fetch, or --no-tags/custom Buildkite checkout config can suppress it. In exactly those setups the old reset --hard "origin/$BRANCH_NAME" could target a stale remote-tracking commit — which is the same class of stale-ref bug the PR set out to fix.
• FETCH_HEAD is written unconditionally by every git fetch to the tip of what was just fetched, so it's independent of refspec configuration. This is strictly safer.
• The intervening git checkout "$BRANCH_NAME" does not touch FETCH_HEAD, and only one branch is fetched, so FETCH_HEAD resolves unambiguously to the freshly-fetched commit at the reset. Correct.

2. Standardization refactor (27180fb)

• Argument handling changed from [[ -n "${1:-}" ]] to [[ $# -gt 0 ]] + RELEASE_VERSION="${1:?…}". This is a deliberate, well-commented behavior change: an explicitly-passed-but-empty first argument now fails fast as a pipeline misconfiguration rather than silently falling through to env/branch detection. Good.
• New RELEASE_VERSION env precedence branch (elif [[ -n "${RELEASE_VERSION:-}" ]]; then :) matches the documented order-of-precedence in the header. Consistent.
• Error now goes to stderr (>&2) and the message is clearer. Good.
• #!/bin/bash -eu is retained, so a failed reset --hard still aborts the job rather than continuing on a stale commit.

Correctness summary

No bugs, security, or performance concerns. The FETCH_HEAD switch closes an edge case the previous origin/$BRANCH_NAME version left open, and the refactor is behavior-preserving except for the intentional, documented empty-argument hardening.

LGTM ✅

Reminder (from the PR description): hold merge until release-toolkit#763 lands and the gem / Gemfile.lock bump can be included here, so both halves of AINFRA-2725 land together.
• ainfra-2725/reset-release-branch-checkout

[AliSoftware]

@mokagio heads-up — these PRs grew since your review, so they're worth another look rather than assuming they're still the three-line change you saw. Same comment on all 13 repos.

Two commits were added on top of the wording fix you suggested:

1. Reset to FETCH_HEAD rather than origin/$BRANCH_NAME. git fetch origin <branch> always writes FETCH_HEAD, but it only updates refs/remotes/origin/<branch> when the remote's configured fetch refspec covers that branch. With Buildkite's default +refs/heads/*:refs/remotes/origin/* the two are equivalent — but on a clone whose refspec was narrowed after origin/<branch> already existed, the fetch leaves that ref stale and the reset lands on the old commit: the very failure this script exists to prevent, reintroduced through the back door. Resetting to FETCH_HEAD drops the refspec dependency entirely.

2. Standardized the script across all 13 repos. Tangent to the original issue, bundled deliberately. The script had drifted into five different shapes — argument required via ${1?…} or ${1:?…}, argument plus a BUILDKITE_BRANCH fallback, argument with a hand-rolled usage check, the same under a different variable name, and (in simplenote-android) no argument at all, reading RELEASE_VERSION from the environment. Having rolled the same one-line fix out thirteen times this week, that divergence is pure friction.

All 13 now share one canonical script. The resolution order is a superset of what every repo did before — argument, then RELEASE_VERSION from the environment, then the release/* branch the build runs on — so no call site needed changing. The BUILDKITE_BRANCH fallback only fires on a branch matching ^release/, and it derives the branch name back from that same value, so it cannot select a branch other than the one the build was already triggered on.

It also closes two latent bugs: under bash -eu, both [[ -z "${RELEASE_VERSION}" ]] on an unset variable and a bare RELEASE_VERSION=$1 with no arguments abort with unbound variable before their intended usage message can print. And an argument that is passed but empty — what happens when a pipeline forwards an unset $RELEASE_VERSION — is now a hard error everywhere, instead of resolving to release/ or silently falling through to the current branch.

The scripts are byte-identical across all 13 repos and pass shellcheck everywhere; the argument resolution was exercised across every combination (argument / empty argument / environment variable / release/* / trunk / feature branch / unset). Full rationale in the updated PR description.

Worth flagging that the release-toolkit PR also picked up a real fix from your review — the tag-based lookup was preferring a leftover draft over the release that actually owns the tag.

[claude]

Claude finished @AliSoftware's task in 1m 8s —— View job

---

PR Review: Realign the release branch on its remote during checkout

• Read the new commit since the last review (669a448 — Simplify the release version resolution)
• Verify the collapsed argument-resolution logic is correct under bash -eu
• Re-confirm the FETCH_HEAD reset still holds

---

One new commit landed since my last review (669a448). It collapses the previous three-way if/elif/else into a plain assignment plus two guards. I re-read the whole script in its current form — it's correct.

669a448 — resolution logic

RELEASE_VERSION="${1:-${RELEASE_VERSION:-}}"

if [[ -z "$RELEASE_VERSION" && "${BUILDKITE_BRANCH:-}" =~ ^release/ ]]; then
  RELEASE_VERSION="${BUILDKITE_BRANCH#release/}"
fi

if [[ -z "$RELEASE_VERSION" ]]; then
  echo "Error: no release version. ..." >&2
  exit 1
fi

• set -u safety is preserved. The nested ${1:-${RELEASE_VERSION:-}} is the important detail, and the commit message calls it out correctly: the inner :- is what keeps RELEASE_VERSION from being dereferenced-while-unset. Written as ${1:-$RELEASE_VERSION} the default expression itself would trip unbound variable under -u when both are unset. As written, both the arg and the env var are unset-safe. ✅
• Self-referential assignment is fine. RELEASE_VERSION="…${RELEASE_VERSION:-}…" fully evaluates the RHS before assigning, so reading and writing the same name in one statement is well-defined in bash. ✅
• ${BUILDKITE_BRANCH#release/} on line 23 is safe. It's reached only after the =~ ^release/ guard matched, which is impossible for an unset/empty value — so the bare $BUILDKITE_BRANCH (no :-) can't trip -u there. ✅
• Header comment and code now agree. The header says "the first of these that is set and non-empty", and ${1:-…} treats an empty argument as not-set — consistent wording.
• The documented behaviour change is reasonable. An explicitly-passed-but-empty first argument no longer gets a dedicated error; it falls through to env → release/* branch → the generic error. The commit message's justification holds: the only case where the fallback silently "catches" an empty arg is a release/* build, where the derived branch is necessarily the one the build already runs on, so it can't select a different branch than intended. This does trade away the previous fail-fast on empty args, but the reasoning that it can't mis-target is sound.

Everything else (unchanged, still holds)

• git reset --hard FETCH_HEAD (line 38) remains refspec-independent and targets the freshly-fetched commit — the robustness fix from 01700b3.
• Fresh-checkout case is still a genuine no-op.
• #!/bin/bash -eu (line 1) still aborts the job on a failed reset rather than continuing on a stale commit.

Correctness summary

No bugs, security, or performance concerns. The simplification is behaviour-preserving except for the intentional, well-documented empty-argument change, and it stays -u-safe throughout. Nice readability win over the elif … : # no-op form.

LGTM ✅

Reminder (from the PR description): hold merge until release-toolkit#763 lands so the gem / Gemfile.lock bump can be included here and both halves of AINFRA-2725 land together.
• ainfra-2725/reset-release-branch-checkout

[claude]

Claude finished @AliSoftware's task in 30s —— View job

---

PR Review: Realign the release branch on its remote during checkout

• Read the new commit since the last review (6987596 — Reformat the realignment comment as a bullet list)
• Verify the change is comment-only and behaviour-preserving
• Re-confirm the script as a whole still holds

---

One new commit landed since my last review (6987596). It's a comment-only, purely cosmetic change — the two justifications for the reset are split out of a prose paragraph into a bullet list:

# Buildkite can reuse a working copy where "$BRANCH_NAME" was left at a different commit by a previous job,
# so force the local branch to the fetched commit.
#  - `reset --hard` rather than `git pull`, to avoid merging if the two diverged
#  - `FETCH_HEAD` rather than `origin/$BRANCH_NAME`, which `git fetch <branch>` only updates opportunistically.
git reset --hard FETCH_HEAD

No executable lines changed. git diff touches only the three comment lines above line 38 — the diff confirms it (3 insertions, 3 deletions, all comment text). This is a readability win: the two rationales now stand apart instead of wrapping mid-clause, and they mirror the numbered resolution-order comment in the header (lines 7–10).

Correctness (unchanged, still holds)

• git reset --hard FETCH_HEAD (line 38) is refspec-independent and targets the freshly-fetched commit — the robustness fix from 01700b3.
• Resolution logic (lines 20–29) stays set -u-safe via the nested ${1:-${RELEASE_VERSION:-}}, and ${BUILDKITE_BRANCH#release/} on line 23 is only reached after the =~ ^release/ guard matches.
• Fresh-checkout case remains a genuine no-op.
• #!/bin/bash -eu (line 1) still aborts the job on a failed reset rather than continuing on a stale commit.

Summary

No bugs, security, or performance concerns. The change is documentation-only and behaviour-preserving. LGTM ✅

Reminder (from the PR description): hold merge until release-toolkit#763 lands so the gem / Gemfile.lock bump can be included here and both halves of AINFRA-2725 land together.

• ainfra-2725/reset-release-branch-checkout

[mokagio]

Reviewed the updated change on bloom/DayOne-Android#8243. The script is byte-identical across every repo in this rollout (verified: same sha256, same 1665 bytes), so that review applies here unchanged.

Short version: a worthwhile refinement on the original implementation. One non-blocking nitpick — git checkout "$BRANCH_NAME" still fails if the branch is absent locally and the fetch refspec doesn't create origin/$BRANCH_NAME, before the reset can reach FETCH_HEAD; git checkout -B "$BRANCH_NAME" FETCH_HEAD would do both and drop the dependency entirely.

As on #8243, withholding approval until the release-toolkit update lands, to avoid accidental merges as warned in the description.

Posted by Claude Code (Opus 5) on behalf of @mokagio with approval.

[claude]

Claude finished @AliSoftware's task in 1m 9s —— View job

---

PR Review: Realign the release branch on its remote during checkout

• Read the new commit since the last review (251320e — Update release-toolkit to 14.11.2)
• Verify the Gemfile.lock bump is consistent with the Gemfile constraint
• Re-confirm the script and the merge-hold condition

---

The synchronize that triggered this run is the commit everyone was waiting for: 251320e bumps fastlane-plugin-wpmreleasetoolkit to 14.11.2, pulling in release-toolkit#763 — the other half of AINFRA-2725. This is the change the PR description asked to hold merge for, so that merge-hold condition is now satisfied.

251320e — Gemfile.lock

-    fastlane-plugin-wpmreleasetoolkit (14.11.0)
+    fastlane-plugin-wpmreleasetoolkit (14.11.2)

• Consistent with the Gemfile. The pin is gem 'fastlane-plugin-wpmreleasetoolkit', '~> 14.11.0', which resolves to >= 14.11.0, < 14.12.0 — so 14.11.2 is in range and no Gemfile edit was needed. Lockfile-only bump is correct. ✅
• Transitive refreshes look benign — aws-partitions, aws-sdk-s3, googleauth, json are all patch-level moves that bundle update picked up, as the commit message states. No direct dependency changed shape.
• BUNDLED WITH 4.0.15 → 4.0.17 — worth a passing note only: this asks for a newer Bundler at install time. Bundler is forgiving here (it won't hard-fail on a patch mismatch), and CI provisions Bundler from the toolchain rather than pinning 4.0.15, so this is a non-issue in practice. Flagging for visibility, not as a defect.

The 14.11.0 → 14.11.2 jump (skipping .1) is expected — .2 is simply the first release carrying #763; nothing was yanked.

Script (checkout-release-branch.sh) — unchanged, still holds

No change to the script in this push. Re-confirming the state from prior reviews:

• git reset --hard FETCH_HEAD (line 38) is refspec-independent and targets the freshly-fetched commit — the robustness fix from 01700b3.
• Resolution logic (lines 20–29) stays set -u-safe via the nested ${1:-${RELEASE_VERSION:-}}, and ${BUILDKITE_BRANCH#release/} (line 23) is only reached after the =~ ^release/ guard matches.
• Fresh-checkout case remains a genuine no-op; #!/bin/bash -eu (line 1) aborts the job on a failed reset rather than continuing on a stale commit.

On @mokagio's standing nitpick (git checkout -B "$BRANCH_NAME" FETCH_HEAD to collapse checkout+reset and also handle the branch-absent-locally case) — it's a real, minor robustness gain, but non-blocking, and the two-line form is arguably clearer about why the reset exists. Consistent with the author's decision to keep the script byte-identical across all 13 repos.

Summary

No bugs, security, or performance concerns. The lockfile bump is the intended, in-constraint pickup of the release-toolkit fix, and the script is unchanged. Both halves of AINFRA-2725 are now present in this PR, so the previously-noted merge hold is resolved — this is ready pending @mokagio's approval. ✅
• ainfra-2725/reset-release-branch-checkout

[dangermattic]

	1 Warning
⚠️	This PR is assigned to the milestone 8.18. The due date for this milestone has already passed. Please assign it to a milestone with a later deadline or check whether the release for this milestone has already been finished.

Generated by 🚫 Danger

[AliSoftware]

Updated release-toolkit to 14.11.2, the version that shipped wordpress-mobile/release-toolkit#763 — so both halves of AINFRA-2725 are now in this PR and it is no longer blocked. I removed the do-not-merge note from the description accordingly.

That release makes publish_github_release publish the most recently created GitHub Release when several share the same name, instead of whichever one the GitHub API happened to list first. It also fixes the tag-based lookup used by upload_github_release_assets, which could otherwise resolve to a leftover draft rather than the release that actually owns the tag.

Done with a real bundle update fastlane-plugin-wpmreleasetoolkit, so the lock was properly re-resolved rather than hand-edited. Two side effects worth naming:

• A few unrelated transitive gems that had newer releases were refreshed along with it (aws-partitions, aws-sdk-s3, json, and googleauth in one repo).
• BUNDLED WITH moved to the current 4.0.17. I pinned bundler explicitly for the whole batch so every repo lands on the same version — otherwise one of them would have been silently downgraded.