Skip to content

Realign the release branch on its remote during checkout - #23167

Open
AliSoftware wants to merge 9 commits into
trunkfrom
ainfra-2725/reset-release-branch-checkout
Open

Realign the release branch on its remote during checkout#23167
AliSoftware wants to merge 9 commits into
trunkfrom
ainfra-2725/reset-release-branch-checkout

Conversation

@AliSoftware

@AliSoftware AliSoftware commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

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.

A few unrelated transitive gems were refreshed along with it. The repo's pinned Ruby (3.2.2) and bundler (2.6.8) are deliberately left alone — 14.11.2 declares required_ruby_version >= 3.2.2, so no toolchain change is needed.

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

`checkout-release-branch.sh` fetched the release branch then checked it out, but never moved the local branch to the fetched commit. Buildkite cleans the working copy between jobs, yet can reuse it — so a `refs/heads/release/x.y` left behind by an earlier job on the same agent survives, and `git checkout` then just switches to that stale local ref instead of the freshly fetched remote one. Anything running afterwards, such as the version bump and the GitHub Release draft created by `finalize_release`, would target the wrong commit.

Adding `git reset --hard "origin/$BRANCH_NAME"` after the checkout makes the branch match the remote unconditionally. `reset --hard` rather than `git pull`: it needs no extra network round trip and cannot produce a merge if the local and remote refs have diverged.

This is the second part of AINFRA-2725, a follow-up to the WooCommerce iOS 25.1 release incident.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@AliSoftware
AliSoftware requested a review from a team as a code owner July 30, 2026 21:09
@AliSoftware AliSoftware added this to the 27.1 milestone Jul 30, 2026
@wpmobilebot

wpmobilebot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

App Icon📲 You can test the changes from this Pull Request in Jetpack Android by scanning the QR code below to install the corresponding build.

App NameJetpack Android
Build TypeDebug
Versionpr23167-d13de06
Build Number1498
Application IDcom.jetpack.android.prealpha
Commitd13de06
Installation URL2gc008vdl1b2g
Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

@wpmobilebot

wpmobilebot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

App Icon📲 You can test the changes from this Pull Request in WordPress Android by scanning the QR code below to install the corresponding build.

App NameWordPress Android
Build TypeDebug
Versionpr23167-d13de06
Build Number1498
Application IDorg.wordpress.android.prealpha
Commitd13de06
Installation URL3suj86sso9nl8
Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

Comment thread .buildkite/commands/checkout-release-branch.sh Outdated
AliSoftware and others added 3 commits July 31, 2026 19:01
"realign it on the remote" read as though the operation happened *on* the remote, rather than describing what the local branch is realigned against. Say plainly what the reset does instead: force the local branch to the fetched commit.

Wording suggested by @mokagio in review.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`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 the default `+refs/heads/*:refs/remotes/origin/*` that Buildkite sets up, 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"` silently lands on the old commit: exactly the failure this script is meant to prevent, reintroduced through the back door.

Resetting to `FETCH_HEAD` removes the dependency on the refspec entirely — it is whatever the line above just fetched.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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, the divergence is pure friction, so this settles on a single canonical version.

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 ever 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`, `[[ -z "${RELEASE_VERSION}" ]]` on an unset variable and a bare `RELEASE_VERSION=$1` with no arguments both abort with `unbound variable` before their intended usage message can print. And an argument that is passed but empty — which happens when a pipeline forwards an unset `$RELEASE_VERSION` — is now a hard error everywhere, rather than resolving to `release/` or silently falling through to the current branch.

The redundant `echo '--- :git: Checkout Release Branch'` in simplenote-android's pipelines is dropped, since the canonical script prints that group header itself.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@AliSoftware

Copy link
Copy Markdown
Contributor Author

@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.

@AliSoftware
AliSoftware requested a review from mokagio July 31, 2026 19:46
AliSoftware and others added 2 commits July 31, 2026 22:11
Collapse the three-way `if/elif/else` into a plain assignment plus two guards. The `: # Already provided through the pipeline environment` no-op branch existed only to skip reassigning a value that was already correct, which reads oddly for anyone who has not just written it.

The one behavioural difference is that an argument that is passed but empty no longer gets its own dedicated error: it now falls through to the environment variable, then to the `release/*` branch, and finally to the same generic error as the unset case. That is a rare enough situation to not be worth a distinct branch, and when the fallback does catch it, it resolves to the branch the build is already running on, which cannot be a different branch than intended.

Note the nested guard in `${1:-${RELEASE_VERSION:-}}`: written as `${1:-$RELEASE_VERSION}`, the default expression itself dereferences an unset variable, so under `bash -eu` the script would abort with `RELEASE_VERSION: unbound variable` when neither is set — the same latent failure this standardization removed from a couple of the repos.

Also say "a different commit" rather than "an older commit" when describing the reused working copy, since a stale local ref is not necessarily behind the remote.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The two justifications—`reset --hard` over `git pull`, and `FETCH_HEAD` over the remote-tracking ref—were run together in a prose paragraph that wrapped mid-clause, so neither stood out. Split them into bullets under the sentence stating what the reset does.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@mokagio

mokagio commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

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.

AliSoftware and others added 2 commits August 3, 2026 15:09
`bundle update` fails on 3.2.2 while building nokogiri 1.19.4 from source—`gumbo.c: fatal error: 'nokogiri_gumbo.h' file not found`—which blocks picking up any new release-toolkit version. The lockfile pins `PLATFORMS: ruby`, so there is no precompiled gem to fall back on and the native build has to succeed.

3.4.9 is what the rest of the mobile repos already use.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Picks up wordpress-mobile/release-toolkit#763, which makes `publish_github_release` publish the most recently created GitHub Release when several share the same name rather than whichever one the API happened to list first. That is the other half of AINFRA-2725: without it, a re-run of `finalize_release` can still leave the git tag on the wrong commit, which is what caused the WooCommerce iOS 25.1 incident.

`bundle update` also refreshed a few unrelated transitive gems that had newer releases, and bumped `BUNDLED WITH` to the current 4.0.17.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@AliSoftware

Copy link
Copy Markdown
Contributor Author

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.

Reverts the earlier `.ruby-version` bump to 3.4.9 and re-resolves the lockfile with the bundler this repo was already pinned to.

Ruby 3.4.9 is only present on the `xcode-26.6` CI image and newer—26.5 ships 3.4.2, 26.4.1 ships 3.4.0—so pinning it here breaks any job running on an earlier image. The bump was never needed to consume the new toolkit anyway: `fastlane-plugin-wpmreleasetoolkit` 14.11.2 declares `required_ruby_version >= 3.2.2`, exactly as 13.8.1 did. It was only needed to let `bundle update` compile nokogiri's native extension locally, which `bundle lock --update` sidesteps by resolving without installing.

Updating Ruby across the release pipelines is a worthwhile change, but it is a bigger one than it looks and does not belong bundled with a gem bump.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@AliSoftware

Copy link
Copy Markdown
Contributor Author

Correction — I've reverted the .ruby-version bump I pushed earlier, and re-resolved the lockfile with the bundler this repo was already pinned to. The release-toolkit update to 14.11.2 stays; only the toolchain changes are undone.

Ruby 3.4.9 is only present on the xcode-26.6 CI image and newer. Per the VM summaries, 26.5 ships 3.4.2, 26.4.1 ships 3.4.0 and 26.3 ships 3.3.4 — so pinning 3.4.9 breaks any job running on an earlier image.

The bump was never needed to consume the new toolkit in the first place: 14.11.2 declares required_ruby_version >= 3.2.2, exactly as 13.8.1 did. It was only needed so that bundle update could compile nokogiri's native extension on my machine, which bundle lock --update sidesteps by resolving without installing. So this PR now carries the gem bump on the repo's existing Ruby 3.2.2 and bundler 2.6.8, and the diff is limited to what it should have been.

Updating Ruby across the release pipelines is still worth doing — it just deserves its own change rather than riding along with a gem bump.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants